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

["Pankaj Raghav (Samsung)" <pankaj.raghav@linux.dev>]

On Tue, Nov 10, 2026 at 05:00:57PM +0900, 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. The two can differ outside the
> first group.
> 
> Since commit ec8f62cc109a ("xfs: don't limit software atomic writes by
> the group alignment"), a misaligned write can reach the hardware path.
> The block layer can fail it with -EINVAL, and XFS does not retry it
> through the COW path.
> 
> 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 changes look good to me.

It looks like this bug got exposed as a side effect of relaxing the
software atomic writes as we should have always did this check in
xfs_bmap_hw_atomic_write_possible()?

Could you explain a bit more of that in the commit to give some more
context?

Especially, we need mkfs.xfs < 6.16 to trigger this for data volume but
it is an issue for rt volume even on latest xfsprogs.

> ---
> Affects a group size that is not a multiple of the device's atomic
> write unit max. mkfs.xfs 6.16+ rounds agsize to it, not rgsize.
> 
> Tested on emulated NVMe, 64 KiB atomic unit and boundary: 64 KiB
> atomic overwrites in AG 1-3 (mkfs.xfs 6.6.0, agsize=262142b) and
> rtgroups 1-7 (xfsprogs 7ebfc20eb88e, rgsize=65534b) failed with EINVAL
> before and use COW after. fstests -g atomicwrites, xfs/838 (rt): no
> change.
> ---
>  fs/xfs/xfs_iomap.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index c3465204b8d..cfa2f7ac03a 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -848,8 +848,11 @@ 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: br_startblock carries the group number in
> +	 * its high bits, so it can be aligned when the disk address is not.

I guess you can keep the comment generic by saying "fsbno is not a linear
disk address" as we can have start offsets for rt groups.

>  	 */
> -	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)))

Good catch.

-- 
Pankaj