On 06/03/2024 21:31, Dave Chinner wrote:
+ xfs_extlen_t extsz = xfs_get_extsz(ip);
+ struct xfs_buftarg *target = xfs_inode_buftarg(ip);
+ struct block_device *bdev = target->bt_bdev;
+ struct request_queue *q = bdev->bd_queue;
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_sb *sbp = &mp->m_sb;
+ unsigned int awu_min, awu_max;
+ unsigned int extsz_bytes = XFS_FSB_TO_B(mp, extsz);
+
+ awu_min = queue_atomic_write_unit_min_bytes(q);
+ awu_max = queue_atomic_write_unit_max_bytes(q);
We really should be storing these in the xfs_buftarg at mount time,
like we do logical and physical sector sizes.
This has been mentioned previously, and Darrick thought that it was not
safe. Please see first response in:
https://lore.kernel.org/linux-xfs/20231003161029.GG21298@frogsfrogsfrogs/#t
So if this really is true, then I'll stick with something like what I
have here and add a comment on that.
However, in this series the block layer does check for out-of-range
atomic write BIOs in 1/14. So we could store the values in xfs_buftarg,
as you suggest for the lookup here. If the bdev geometry does really
change beneath us, worse thing that happens is that we may report
incorrect values for statx.
Similar to sector
sizes, they*must not change* once the filesystem has been created
on the device, let alone during an active mount. The whole point of
the xfs_buftarg is to store the information the filesystem
needs to do IO to the underlying block device so we don't have to
chase pointers deep into the block device whenever we need to use
static geometry information.....
+ if (sbp->sb_blocksize > awu_max || awu_min > sbp->sb_blocksize ||
+ !xfs_inode_atomicwrites(ip)) {
+ *unit_min = 0;
+ *unit_max = 0;
+ return;
+ }
Again, this is comparing static geometry - if the block size doesn't
allow atomic writes, then the inode flag should never be set. i.e.
geometry is checked when configuring atomic writes, not in every
place we need to check if atomic writes are supported. Hence this
should simply be:
if (!xfs_inode_has_atomic_writes(ip)) {
*unit_min = 0;
*unit_max = 0;
return;
} >
before we even look at the xfs_buftarg to get the supported min/max
values for the given device.
Thanks,
John
