[cc linux-ext4 because ext4_file_write_iter has the same problem]
On Tue, Jul 22, 2025 at 03:00:16PM +0000, John Garry wrote:
> Atomic writes are not currently supported for DAX, but two problems exist:
> - we may go down DAX write path for IOCB_ATOMIC, which does not handle
> IOCB_ATOMIC properly
> - we report non-zero atomic write limits in statx (for DAX inodes)
>
> We may want atomic writes support on DAX in future, but just disallow for
> now.
>
> For this, ensure when IOCB_ATOMIC is set that we check the write size
> versus the atomic write min and max before branching off to the DAX write
> path. This is not strictly required for DAX, as we should not get this far
> in the write path as FMODE_CAN_ATOMIC_WRITE should not be set.
>
> In addition, due to reflink being supported for DAX, we automatically get
> CoW-based atomic writes support being advertised. Remedy this by
> disallowing atomic writes for a DAX inode for both sw and hw modes.
You might want to add a separate patch to insert:
if (WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC))
return -EIO;
into dax_iomap_rw to make it clear that DAX doesn't support ATOMIC
writes.
> Reported-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> Fixes: 9dffc58f2384 ("xfs: update atomic write limits")
> Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
Otherwise seems reasonable to me...
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
--D
> ---
> Difference to v1:
> - allow use max atomic mount option and always dax together (Darrick)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index ed69a65f56d7..979abcb25bc7 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1099,9 +1099,6 @@ xfs_file_write_iter(
> if (xfs_is_shutdown(ip->i_mount))
> return -EIO;
>
> - if (IS_DAX(inode))
> - return xfs_file_dax_write(iocb, from);
> -
> if (iocb->ki_flags & IOCB_ATOMIC) {
> if (ocount < xfs_get_atomic_write_min(ip))
> return -EINVAL;
> @@ -1114,6 +1111,9 @@ xfs_file_write_iter(
> return ret;
> }
>
> + if (IS_DAX(inode))
> + return xfs_file_dax_write(iocb, from);
> +
> if (iocb->ki_flags & IOCB_DIRECT) {
> /*
> * Allow a directio write to fall back to a buffered
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 07fbdcc4cbf5..bd6d33557194 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -358,9 +358,20 @@ static inline bool xfs_inode_has_bigrtalloc(const struct xfs_inode *ip)
>
> static inline bool xfs_inode_can_hw_atomic_write(const struct xfs_inode *ip)
> {
> + if (IS_DAX(VFS_IC(ip)))
> + return false;
> +
> return xfs_inode_buftarg(ip)->bt_awu_max > 0;
> }
>
> +static inline bool xfs_inode_can_sw_atomic_write(const struct xfs_inode *ip)
> +{
> + if (IS_DAX(VFS_IC(ip)))
> + return false;
> +
> + return xfs_can_sw_atomic_write(ip->i_mount);
> +}
> +
> /*
> * In-core inode flags.
> */
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 149b5460fbfd..603effabe1ee 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -616,7 +616,8 @@ xfs_get_atomic_write_min(
> * write of exactly one single fsblock if the bdev will make that
> * guarantee for us.
> */
> - if (xfs_inode_can_hw_atomic_write(ip) || xfs_can_sw_atomic_write(mp))
> + if (xfs_inode_can_hw_atomic_write(ip) ||
> + xfs_inode_can_sw_atomic_write(ip))
> return mp->m_sb.sb_blocksize;
>
> return 0;
> @@ -633,7 +634,7 @@ xfs_get_atomic_write_max(
> * write of exactly one single fsblock if the bdev will make that
> * guarantee for us.
> */
> - if (!xfs_can_sw_atomic_write(mp)) {
> + if (!xfs_inode_can_sw_atomic_write(ip)) {
> if (xfs_inode_can_hw_atomic_write(ip))
> return mp->m_sb.sb_blocksize;
> return 0;
> --
> 2.43.5
>
