Re: (2) [PATCH v2] xfs: check the disk address for hardware atomic writes

[John Garry <john.garry@linux.dev>]

On 11/13/26 02:55, Daejun Park wrote:
> On Thu, Nov 12, 2026 at 08:29:42AM +0000, John Garry wrote:
>> On 11/12/26 00:41, Daejun Park via B4 Relay wrote:
> [...]
>>> -	if (!IS_ALIGNED(imap->br_startblock, imap->br_blockcount))
>> Is there anywhere else in which we do an alignment check on
>> br_startblock that relies on the group size to be a multiple of the HW
>> awu max?
> No, I did not find another one. This is the only check of the start
> block for the REQ_ATOMIC path. Two other places depend on the group
> alignment, but neither checks br_startblock:
> 
> - xfs_get_atomic_write_max_opt() already caps max_opt at
>    xfs_calc_group_awu_align_max(), the largest power-of-two factor of
>    the group size (from ec8f62cc109a).
> - The allocator aligns the AG block number, not the disk address,
>    for example for XFS_BMAPI_EXTSZALIGN in
>    xfs_atomic_write_cow_iomap_begin(). In an unaligned AG, that extent
>    can still be misaligned on disk. With this patch, the next atomic
>    write to it goes through COW instead of failing.

hmm.. I am still not confident that this the best fix. And if it is, 
then I think that we should still change the size pre-check in 
xfs_file_dio_write_atomic(). Here's a combined change which I am 
thinking of:

--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -776,7 +776,7 @@ xfs_file_dio_write_atomic(
          * HW offload should be faster, so try that first if it is already
          * known that the write length is not too large.
          */
-       if (ocount > xfs_inode_buftarg(ip)->bt_awu_max)
+       if (ocount > xfs_get_atomic_write_max_opt())
                 dops = &xfs_atomic_write_cow_iomap_ops;
         else
                 dops = &xfs_direct_write_iomap_ops;
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index c3465204b8d5..c54aaa4b8816 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -863,7 +863,7 @@ xfs_bmap_hw_atomic_write_possible(
         /*
          * The ->iomap_begin caller should ensure this, but check anyway.
          */
-       return len <= xfs_inode_buftarg(ip)->bt_awu_max;
+       return len <= xfs_get_atomic_write_max_opt();
  }

bt_awu_max is the HW max, and you are saying that it is not aligned with 
the AG size (and so need a disk alignment check). However, opt max 
should be (aligned to AG size), so we could check that vs size instead.

Note that I am just using xfs_get_atomic_write_max_opt() for 
illustrative purposes and we could save this value in xfs_inode_buftarg().

BTW, with your proposed change, since HW size is not aligned to AG size 
and we only check the size vs HW size for HW offload, it is in theory 
possible to have an atomic write which straddles the AG and is rejected, 
right?

Thanks,
John