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

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

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,