linux-kernel - Re: [RFC PATCH] nvmet: Make blksize

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6281253.5x4bddO4Km@anvil>
Date: Fri, 18 Apr 2025 12:53:28 +0200
From: Richard Weinberger <richard@...ma-star.at>
To: linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Damien Le Moal <dlemoal@...nel.org>
Cc: Richard Weinberger <richard@....at>, kch@...dia.com, sagi@...mberg.me,
 hch@....de, upstream+nvme@...ma-star.at
Subject: Re: [RFC PATCH] nvmet: Make blksize_shift configurable

On Freitag, 18. April 2025 12:23 Damien Le Moal wrote:
> On 4/18/25 18:56, Richard Weinberger wrote:
> > On Freitag, 18. April 2025 11:37 'Damien Le Moal' via upstream wrote:
> >>> +	if (!ns->blksize_shift)
> >>> +		ns->blksize_shift = blksize_bits(bdev_logical_block_size(ns->bdev));
> >>
> >> If the user set logical block size is smaller than the block dev logical block
> >> size, this is not going to work... No ? Am I missing something ?
> > 
> > Likely, yes.
> > TBH, I'm not sure whether it makes actually sense for the bdev case to make
> > blksize_shift configurable.
> 
> Probably not... I do understand the value for the file case though.

The use case is exposing ready-to-use cloud images like:
https://cloud.debian.org/images/cloud/bookworm/20250416-2084/debian-12-generic-amd64-20250416-2084.raw

via NVme-of TCP.
Yesterday I did so and figured that no GPT partitions got detected because of different block sizes.
Setting the block size in nvmet to 512 made it work.

If there are better ways to achieve the same, I'm open for suggestions.

> 
> > The case I see most benefit is the backing file case.
> > 
> >>> +	if (!ns->blksize_shift) {
> >>> +		/*
> >>> +		 * i_blkbits can be greater than the universally accepted
> >>> +		 * upper bound, so make sure we export a sane namespace
> >>> +		 * lba_shift.
> >>> +		 */
> >>> +		ns->blksize_shift = min_t(u8,
> >>> +				file_inode(ns->file)->i_blkbits, 12);
> >>
> >> This will work for any block size, regardless of the FS block size, but only if
> >> ns->buffered_io is true. Doesn't this require some more checks with regards to
> >> O_DIRECT (!ns->buffered_io case) ?
> > 
> > Good catch. I'll add a check.
> 
> And by the way, you need to check for STATX_DIOALIGN since some FS (e.g. xfs)
> can handle direct IOs that are not aligned to the FS block size. See the recent
> changes in drivers/block/loop.c to improve direct IO handling, specifically, the
> function loop_query_min_dio_size().

Ok!

Thanks,
//richard

-- 
sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, AUT UID/VAT Nr:
ATU 66964118 | FN: 374287y