On 21/02/2024 17:00, Darrick J. Wong wrote:
Hmm. Well, if we move towards pushing all the hardware checks out of
xfs/iomap and into whatever goes on underneath submit_bio then I guess
we don't need to check device support here at all.
Yeah, I have been thinking about this. But I was still planning on putting a
"bdev on atomic write" check here, as you mentioned.
But is this a proper method to access the bdev for an xfs inode:
STATIC bool
xfs_file_can_atomic_write(
struct xfs_inode *inode)
{
struct xfs_buftarg *target = xfs_inode_buftarg(inode);
struct block_device *bdev = target->bt_bdev;
if (!xfs_inode_atomicwrites(inode))
return false;
return bdev_can_atomic_write(bdev);
}
There's still a TOCTOU race problem if the bdev gets reconfigured
between xfs_file_can_atomic_write and submit_bio.
If that is the case then a check in the bio submit path is required to
catch any such reconfigure problems - and we effectively have that in
this series.
I am looking at change some of these XFS bdev_can_atomic_write() checks,
but would still have a check in the bio submit path.
However, if you're only using this to advertise the capability via statx
then I suppose that's fine -- userspace has to have some means of
discovering the ability at all. Userspace is also inherently racy.
I do notice the dax check in xfs_bmbt_to_iomap() when assigning iomap->bdev,
which is creating some doubt?
Do you mean this?
if (mapping_flags & IOMAP_DAX)
iomap->dax_dev = target->bt_daxdev;
else
iomap->bdev = target->bt_bdev;
The dax path wants dax_dev set so that it can do the glorified memcpy
operation, and it doesn't need (or want) a block device.
Yes, so proper to use target->bt_bdev for checks for bdev atomic write
capability, right?
Thanks,
John
