Re: [PATCH v2] xfs: fix unaligned access to rmapbt high keys in scrub

[Dave Chinner <dgc@kernel.org>]

On Tue, Nov 10, 2026 at 11:39:27PM +0100, Stian Halseth wrote:
> An rmapbt node stores each key as a low/high pair of packed 20-byte
> struct xfs_rmap_key, so every high key is only 4-byte aligned.
> xchk_rmapbt_check_unwritten_in_keyflags() reads rm_offset through
> union xfs_btree_key, which is 8-byte aligned, so the compiler may use
> an 8-byte load.

Ok, this trips my 'written by an LLM' filter. i.e. long
run-on sentences, strung together with commas and "so ...". I
mention this because....

> diff --git a/fs/xfs/scrub/rmap.c b/fs/xfs/scrub/rmap.c
> index 68e2847c962bb..29609c5967fe3 100644
> --- a/fs/xfs/scrub/rmap.c
> +++ b/fs/xfs/scrub/rmap.c
> @@ -149,8 +149,7 @@ xchk_rmapbt_check_unwritten_in_keyflags(
>  	struct xfs_scrub	*sc = bs->sc;
>  	struct xfs_btree_cur	*cur = bs->cur;
>  	struct xfs_btree_block	*keyblock;
> -	union xfs_btree_key	*lkey, *hkey;
> -	__be64			badflag = cpu_to_be64(XFS_RMAP_OFF_UNWRITTEN);
> +	const struct xfs_rmap_key *lkey, *hkey;
>  	unsigned int		level;
>  
>  	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_PREEN)
> @@ -166,15 +165,15 @@ xchk_rmapbt_check_unwritten_in_keyflags(
>  
>  		keyblock = xfs_btree_get_block(cur, level, &bp);
>  		for (ptr = 1; ptr <= be16_to_cpu(keyblock->bb_numrecs); ptr++) {
> -			lkey = xfs_btree_key_addr(cur, ptr, keyblock);
> +			lkey = &xfs_btree_key_addr(cur, ptr, keyblock)->rmap;

The code fix smells of LLM, too: technically correct, but unreadable
and unmaintainable.

Regardless of where it came from, it's objectively -poor code-
because:

- it uses a pattern that is not found anywhere else in the XFS code.
- it costs nothing to use two variables here which would make the
  code much more readable for a maintainer.
- no comment to explain the issue the code has to avoid.

At minimum, a comment explaining why the code needs to be structured
this way is needed. Humans like me are going to forget why the code
needed to be structured this way in short order. Hence a comment is
needed to remind me why the code was written this way next time I
look at it.

IOWs, the code should look something like this:

			/*
			 * The on-disk high key in the rmap block may not be naturally
			 * aligned, so we have to tell the compiler that we are
			 * decoding rmap keys so it treats them correctly as an
			 * unaligned packed structure.
			 */
			union xfs_btree_key *key;
			const struct xfs_rmap_key *rkey;

			key = xfs_btree_key_addr(cur, ptr, keyblock);
			rkey = &key->rmap;
			if (be64_to_cpu(rkey->rm_offset) & XFS_RMAP_OFF_UNWRITTEN) {
				xchk_btree_set_preen(sc, cur, level);
				break;
			}

			key = xfs_btree_high_key_addr(cur, ptr, keyblock);
			rkey = &key->rmap;
			if (be64_to_cpu(rkey->rm_offset) & XFS_RMAP_OFF_UNWRITTEN) {
				xchk_btree_set_preen(sc, cur, level);
				break;
			}


-Dave.


-- 
Dave Chinner
dgc@kernel.org