Re: [PATCH v5 7/7] xfs: Support setting FMODE_CAN_ATOMIC_WRITE

From: John Garry
Date: Fri Aug 23 2024 - 06:42:43 EST


On 22/08/2024 21:44, Darrick J. Wong wrote:
Do you mean that add a new member to xfs_inode to record this? If yes, it
sounds ok, but we need to maintain consistency (of that member) whenever
anything which can affect it changes, which is always a bit painful.
I actually meant something more like:

static bool
xfs_file_open_can_atomicwrite(
struct file *file,
struct inode *inode)
{
struct xfs_inode *ip = XFS_I(inode);
struct xfs_mount *mp = ip->i_mount;
struct xfs_buftarg *target = xfs_inode_buftarg(ip);

if (!(file->f_flags & O_DIRECT))
return false;
if (!xfs_inode_has_atomicwrites(ip))
return false;
if (mp->m_dalign && (mp->m_dalign % ip->i_extsize))
return false;
if (mp->m_swidth && (mp->m_swidth % ip->i_extsize))
return false;
if (mp->m_sb.sb_blocksize < target->bt_bdev_awu_min)
return false;
if (xfs_inode_alloc_unitsize(ip) > target->bt_bdev_awu_max)
return false;
return true;
}

ok, but we should probably factor out some duplicated code with helpers, like:

bool xfs_validate_atomicwrites_extsize(struct xfs_mount *mp, uint32_t extsize)
{
if (!is_power_of_2(extsize))
return false;

/* Required to guarantee data block alignment */
if (mp->m_sb.sb_agblocks % extsize)
return false;

/* Requires stripe unit+width be a multiple of extsize */
if (mp->m_dalign && (mp->m_dalign % extsize))
return false;

if (mp->m_swidth && (mp->m_swidth % extsize))
return false;

return true;
}


bool xfs_inode_has_atomicwrites(struct xfs_inode *ip)
{
struct xfs_mount *mp = ip->i_mount;
struct xfs_buftarg *target = xfs_inode_buftarg(ip);

if (!(ip->i_diflags2 & XFS_DIFLAG2_ATOMICWRITES))
return false;
if (!xfs_validate_atomicwrites_extsize(mp, ip->i_extsize))
return false;
if (mp->m_sb.sb_blocksize < target->bt_bdev_awu_min)
return false;
if (xfs_inode_alloc_unitsize(ip) > target->bt_bdev_awu_max)
return false;
return true;
}


static bool xfs_file_open_can_atomicwrite(
struct inode *inode,
struct file *file)
{
struct xfs_inode *ip = XFS_I(inode);

if (!(file->f_flags & O_DIRECT))
return false;
return xfs_inode_has_atomicwrites(ip);
}

Those helpers can be re-used in xfs_inode_validate_atomicwrites() and xfs_ioctl_setattr_atomicwrites().


John