On Wed, Jun 18, 2025 at 07:15:09AM +0200, Christoph Hellwig wrote: > On Wed, Jun 18, 2025 at 09:51:10AM +1000, Dave Chinner wrote: > > On Tue, Jun 17, 2025 at 12:52:05PM +0200, Christoph Hellwig wrote: > > > The file system only has a single file system sector size. > > > > 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. If the above filesystem was moved to new devices where the external log device also had a minimum LBA sector size of 4kB, that filesystem must not mount. The 512 byte sector size for the journal means journal IO is aligned and rounded to 512 byte boundaries, not 4kB boundaries like the new underlying device requires. IOWs, that fs config is unusable on that new device config. This sort of sector size mismatch is currently caught by the xfs_setup_devices() -> xfs_configure_buftarg() -> bdev_validate_blocksize() path. That validation path is what you are removing in this series, so you are introducing regressions in device sector size validation during mount.... > Even then it would be better to do this directly based off the superblock > and not use a field in the buftarg currently only used for cached buffers > (which aren't used on anything but the main device). IDGI. The sector size passed to xfs_configure_buftarg() by xfs_setup_devices() for the bdev LBA size check comes directly from the superblock. You're advocating that we do exactly what the code you are removing already does.... -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
