Re: [PATCH 7/7] xfs: remove the bt_meta_sectorsize field in struct buftarg

[Dave Chinner <david@fromorbit.com>]

On Tue, Jun 24, 2025 at 04:11:17PM +0200, Christoph Hellwig wrote:
> On Thu, Jun 19, 2025 at 12:42:39PM +1000, Dave Chinner wrote:
> > > > The external log device can have a different sector size to
> > > > the rest of the filesystem. This series looks like it removes the
> > > > ability to validate that the log device sector size in teh
> > > > superblock is valid for the backing device....
> > > 
> > > I don't follow.  Do you mean it remove the future possibility to do this?
> > 
> > No, I mean that this:
> > 
> > # mkfs.xfs -l sectsize=512,logdev=/dev/nvme1n1 -d sectsize=4k ....  /dev/nvme0n1
> > 
> > is an valid filesystem configuration and has been for a long, long
> > time. i.e. the logdev does not have to have the same physical sector
> > size support as the data device.
> 
> Sure, and I've never disagreed.  But you'd not explained how that is
> relevant for this patch.  The bt_meta_sectorsize is only used for
> asserting the alignment of cached buffers, and we place no buffers
> (cached or uncached) on the log device ever.

Again: I am not talking about the bt_meta_sectorsize removal being a
problem!

I will repeat it once more: this patchset removes the check that
guarantees the the underlying block device has a sector size that is
valid for the sector size the filesystem devices are configured to
use. That is not acceptible - a 512 byte sector filesystem device
must not be able to mount on a hard 4kB sector device because the
moment we do a 512 byte aligned IO to the log device, the bdev will
give an EIO error and we'll shut down the filesystem.

We check for sector size mismatches at device open time so we don't
get into this nasty situation. You are removing those sector size
checks in this patch set. This is an obvious functional regression,
and that is the part of this change that I'm NACKing. 

Christoph, I explained this in the email you are replying to, and
you have simply ignored it all. Please at least read the entire
email before you respond this time!

-Dave.
-- 
Dave Chinner
david@fromorbit.com