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@xxxxxxxxxxxxx
