[PATCH v2] xfs: check the disk address for hardware atomic writes

[PATCH v2] xfs: check the disk address for hardware atomic writes

[Daejun Park via B4 Relay]

From: Daejun Park <daejun7.park@samsung.com>

xfs_bmap_hw_atomic_write_possible() checks the alignment of the encoded
br_startblock, not of the disk address. This was harmless while atomic
writes were capped at the greatest power-of-two factor of the group
size.

Commit ec8f62cc109a ("xfs: don't limit software atomic writes by the
group alignment") lifted that cap. The block layer can now fail a
misaligned write with -EINVAL, and XFS does not fall back to COW.
Unaligned AGs, such as those made by mkfs.xfs before 6.16, and rtgroups
made by any version can hit this.

Check the disk address instead.

Fixes: ec8f62cc109a ("xfs: don't limit software atomic writes by the group alignment")
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
The bug needs a group size that is not a multiple of the device's
atomic write unit max. mkfs.xfs 6.16+ rounds agsize to a multiple
of it if the device reports one at mkfs time, but never rounds
rgsize. Before 6.16, a single device of 4 TB or more got an odd
agsize.

Tested on emulated NVMe with a 64 KiB atomic unit and boundary.
64 KiB atomic overwrites in AG 1-3 (mkfs.xfs 6.6.0 -d agsize=262142b)
and rtgroups 1-7 (xfsprogs 7ebfc20eb88e -r rgsize=65534b) failed with
EINVAL before and used COW after. fstests -g atomicwrites and xfs/838
(rt) give the same results before and after (generic/773 fails with
ENOSPC on both).

Changes in v2:
- Explain why the check was harmless before ec8f62cc109a and which
  filesystems are affected (Pankaj).
- Reword the comment: an fsbno is not always a linear disk address
  (Pankaj).
---
 fs/xfs/xfs_iomap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index c3465204b8d..119ff8e4c95 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -848,8 +848,10 @@ xfs_bmap_hw_atomic_write_possible(
 	 * atomic writes are required to be naturally aligned for disk blocks,
 	 * which ensures that we adhere to block layer rules that we won't
 	 * straddle any boundary or violate write alignment requirement.
+	 * Check the disk address: an fsbno is not always a linear disk address.
 	 */
-	if (!IS_ALIGNED(imap->br_startblock, imap->br_blockcount))
+	if (!IS_ALIGNED(xfs_fsb_to_db(ip, imap->br_startblock),
+			XFS_FSB_TO_BB(mp, imap->br_blockcount)))
 		return false;
 
 	/*

---
base-commit: 48326a2382d80715475d10ce98734ac65d2d2150
change-id: 20261110-xfs-hw-atomic-align-0eab6833447a

Best regards,
-- 
Daejun Park <daejun7.park@samsung.com>

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

[John Garry]

On 11/12/26 00:41, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@samsung.com>
> 
> xfs_bmap_hw_atomic_write_possible() checks the alignment of the encoded
> br_startblock, not of the disk address. This was harmless while atomic
> writes were capped at the greatest power-of-two factor of the group
> size.
> 
> Commit ec8f62cc109a ("xfs: don't limit software atomic writes by the
> group alignment") lifted that cap. The block layer can now fail a
> misaligned write with -EINVAL, 

It's a good thing that we have those block layer checks...

> and XFS does not fall back to COW.
> Unaligned AGs, such as those made by mkfs.xfs before 6.16, and rtgroups
> made by any version can hit this.
> 
> Check the disk address instead.
> 
> Fixes: ec8f62cc109a ("xfs: don't limit software atomic writes by the group alignment")
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> ---
> The bug needs a group size that is not a multiple of the device's
> atomic write unit max. mkfs.xfs 6.16+ rounds agsize to a multiple
> of it if the device reports one at mkfs time, but never rounds
> rgsize. Before 6.16, a single device of 4 TB or more got an odd
> agsize.
> 
> Tested on emulated NVMe with a 64 KiB atomic unit and boundary.
> 64 KiB atomic overwrites in AG 1-3 (mkfs.xfs 6.6.0 -d agsize=262142b)
> and rtgroups 1-7 (xfsprogs 7ebfc20eb88e -r rgsize=65534b) failed with
> EINVAL before and used COW after. fstests -g atomicwrites and xfs/838
> (rt) give the same results before and after (generic/773 fails with
> ENOSPC on both).
> 
> Changes in v2:
> - Explain why the check was harmless before ec8f62cc109a and which
>    filesystems are affected (Pankaj).
> - Reword the comment: an fsbno is not always a linear disk address
>    (Pankaj).
> ---
>   fs/xfs/xfs_iomap.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index c3465204b8d..119ff8e4c95 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -848,8 +848,10 @@ xfs_bmap_hw_atomic_write_possible(
>   	 * atomic writes are required to be naturally aligned for disk blocks,
>   	 * which ensures that we adhere to block layer rules that we won't
>   	 * straddle any boundary or violate write alignment requirement.
> +	 * Check the disk address: an fsbno is not always a linear disk address.
>   	 */
> -	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?

> +	if (!IS_ALIGNED(xfs_fsb_to_db(ip, imap->br_startblock),
> +			XFS_FSB_TO_BB(mp, imap->br_blockcount)))
>   		return false;
>   
>   	/*
> 
> ---
> base-commit: 48326a2382d80715475d10ce98734ac65d2d2150
> change-id: 20261110-xfs-hw-atomic-align-0eab6833447a
> 
> Best regards,

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

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

[Pankaj Raghav (Samsung)]

On Thu, Nov 12, 2026 at 09:41:56AM +0900, Daejun Park wrote:
> xfs_bmap_hw_atomic_write_possible() checks the alignment of the encoded
> br_startblock, not of the disk address. This was harmless while atomic
> writes were capped at the greatest power-of-two factor of the group
> size.
> 
> Commit ec8f62cc109a ("xfs: don't limit software atomic writes by the
> group alignment") lifted that cap. The block layer can now fail a
> misaligned write with -EINVAL, and XFS does not fall back to COW.
> Unaligned AGs, such as those made by mkfs.xfs before 6.16, and rtgroups
> made by any version can hit this.
> 
> Check the disk address instead.
> 
> Fixes: ec8f62cc109a ("xfs: don't limit software atomic writes by the group alignment")
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Looks good,
Reviewed-by: Pankaj Raghav (Samsung) <pankaj.raghav@linux.dev>

-- 
Pankaj

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

[Daejun Park]

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.

Best regards,
Daejun

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

[John Garry]

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