On Tue, Apr 22, 2025 at 12:27:29PM +0000, John Garry wrote:
> Currently only HW which can write at least 1x block is supported.
>
> For supporting atomic writes > 1x block, a CoW-based method will also be
> used and this will not be resticted to using HW which can write >= 1x
> block.
>
> However for deciding if HW-based atomic writes can be used, we need to
> start adding checks for write length < HW min, which complicates the code.
> Indeed, a statx field similar to unit_max_opt should also be added for this
> minimum, which is undesirable.
>
> HW which can only write > 1x blocks would be uncommon and quite weird, so
> let's just not support it.
>
> Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
> ---
> fs/xfs/xfs_inode.h | 17 ++++++++---------
> fs/xfs/xfs_mount.c | 14 ++++++++++++++
> fs/xfs/xfs_mount.h | 4 ++++
> 3 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index cff643cd03fc..725cd7c16a6e 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -355,20 +355,19 @@ static inline bool xfs_inode_has_bigrtalloc(const struct xfs_inode *ip)
> #define xfs_inode_buftarg(ip) \
> (XFS_IS_REALTIME_INODE(ip) ? \
> (ip)->i_mount->m_rtdev_targp : (ip)->i_mount->m_ddev_targp)
> +/*
> + * Return max atomic write unit for a given inode.
> + */
> +#define xfs_inode_hw_atomicwrite_max(ip) \
> + (XFS_IS_REALTIME_INODE(ip) ? \
> + (ip)->i_mount->m_rt_awu_hw_max : \
> + (ip)->i_mount->m_dd_awu_hw_max)
>
> static inline bool
> xfs_inode_can_hw_atomicwrite(
> struct xfs_inode *ip)
> {
> - struct xfs_mount *mp = ip->i_mount;
> - struct xfs_buftarg *target = xfs_inode_buftarg(ip);
> -
> - if (mp->m_sb.sb_blocksize < target->bt_bdev_awu_min)
> - return false;
> - if (mp->m_sb.sb_blocksize > target->bt_bdev_awu_max)
> - return false;
> -
> - return true;
> + return xfs_inode_hw_atomicwrite_max(ip);
> }
>
> /*
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 00b53f479ece..ee68c026e6cd 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -1082,6 +1082,20 @@ xfs_mountfs(
> xfs_zone_gc_start(mp);
> }
>
> + /*
> + * Set atomic write unit max for mp. Ignore devices which cannot atomic
> + * a single block, as they would be uncommon and more difficult to
> + * support.
> + */
> + if (mp->m_ddev_targp->bt_bdev_awu_min <= mp->m_sb.sb_blocksize &&
> + mp->m_ddev_targp->bt_bdev_awu_max >= mp->m_sb.sb_blocksize)
> + mp->m_dd_awu_hw_max = mp->m_ddev_targp->bt_bdev_awu_max;
If we don't want to use the device's atomic write capabilities due to
fsblock alignment problems, why not just zero out bt_bdev_awu_min/max?
That would cut down on the number of "awu" variables around the
codebase.
/*
* Ignore hardware atomic writes if the device can't handle a single
* fsblock for us. Most devices set the min_awu to the LBA size, but
* the spec allows for a functionality gap.
*/
static void
xfs_buftarg_reconcile_awu(
struct xfs_buftarg *btp)
{
struct xfs_mount *mp = btp->bt_mount;
if (btp->bt_bdev_awu_min > mp->m_sb.sb_blocksize ||
btp->bt_bdev_awu_max < mp->m_sb.sb_blocksize) {
btp->bt_bdev_awu_min = 0;
btp->bt_bdev_awu_max = 0;
}
}
xfs_buftarg_reconcile_awu(mp->m_ddev_targp);
if (mp->m_rtdev_targp)
xfs_buftarg_reconcile_awu(mp->m_rtdev_targp);
Hrm?
--D
> +
> + if (mp->m_rtdev_targp &&
> + mp->m_rtdev_targp->bt_bdev_awu_min <= mp->m_sb.sb_blocksize &&
> + mp->m_rtdev_targp->bt_bdev_awu_max >= mp->m_sb.sb_blocksize)
> + mp->m_rt_awu_hw_max = mp->m_rtdev_targp->bt_bdev_awu_max;
> +
> return 0;
>
> out_agresv:
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index e5192c12e7ac..2819e160f0e9 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -231,6 +231,10 @@ typedef struct xfs_mount {
> unsigned int m_max_open_zones;
> unsigned int m_zonegc_low_space;
>
> + /* ddev and rtdev HW max atomic write size */
> + unsigned int m_dd_awu_hw_max;
> + unsigned int m_rt_awu_hw_max;
> +
> /*
> * Bitsets of per-fs metadata that have been checked and/or are sick.
> * Callers must hold m_sb_lock to access these two fields.
> --
> 2.31.1
>
>
