On Wed, Mar 05, 2025 at 08:18:29AM +0100, Hannes Reinecke wrote:
> On 3/5/25 02:53, Luis Chamberlain wrote:
> > The commit titled "block/bdev: lift block size restrictions to 64k"
> > lifted the block layer's max supported block size to 64k inside the
> > helper blk_validate_block_size() now that we support large folios.
> > However in lifting the block size we also removed the silly use
> > cases many filesystems have to use sb_set_blocksize() to *verify*
> > that the block size < PAGE_SIZE. The call to sb_set_blocksize() can
> > happen in-kernel given mkfs utilities *can* create for example an
> > ext4 32k block size filesystem on x86_64, the issue we want to prevent
> > is mounting it on x86_64 unless the filesystem supports LBS.
> >
> > While, we could argue that such checks should be filesystem specific,
> > there are much more users of sb_set_blocksize() than LBS enabled
> > filesystem on linux-next, so just do the easier thing and bring back
> > the PAGE_SIZE check for sb_set_blocksize() users.
> >
> > This will ensure that tests such as generic/466 when run in a loop
> > against say, ext4, won't try to try to actually mount a filesystem with
> > a block size larger than your filesystem supports given your PAGE_SIZE
> > and in the worst case crash.
> >
> > Cc: Kent Overstreet <kent.overstreet@xxxxxxxxx>
> > Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> > ---
> >
> > Christian, a small fixup for a crash when running generic/466 on ext4
> > in a loop. The issue is obvious, and we just need to ensure we don't
> > break old filesystem expectations of sb_set_blocksize().
> >
> > This still allows XFS with 32k block size and I even tested with XFS
> > with 32k block size and a 32k sector size set.
> >
> > block/bdev.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/block/bdev.c b/block/bdev.c
> > index 3bd948e6438d..de9ebc3e5d15 100644
> > --- a/block/bdev.c
> > +++ b/block/bdev.c
> > @@ -181,7 +181,7 @@ EXPORT_SYMBOL(set_blocksize);
> > int sb_set_blocksize(struct super_block *sb, int size)
> > {
> > - if (set_blocksize(sb->s_bdev_file, size))
> > + if (size > PAGE_SIZE || set_blocksize(sb->s_bdev_file, size))
> > return 0;
> > /* If we get here, we know size is validated */
> > sb->s_blocksize = size;
>
> Can you add a comment stating why it's needed, even with LBS?
> It's kinda non-obious, and we don't want to repeat the mistake
> in the future.
Sure.
Luis
