Re: [PATCH v3 2/6] xfs: add helpers to pack AG prediction info for per-folio tracking

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Feb 03, 2026 at 12:45:33PM +0530, Kundan Kumar wrote:
> On 1/29/2026 6:15 AM, Darrick J. Wong wrote:
> > On Fri, Jan 16, 2026 at 03:38:14PM +0530, Kundan Kumar wrote:
> >> Introduce helper routines to pack and unpack AG prediction metadata
> >> for folios. This provides a compact and self-contained representation
> >> for AG tracking.
> >>
> >> The packed layout uses:
> >>   - bit 31	: valid
> >>   - bit 24-30	: iomap type
> >>   - bit 0-23	: AG number
> > 
> > There are only 5 iomap types, why do you need 7 bits for that?
> > 
> > Also, can you store more bits on a 64-bit system to avoid truncating the
> > AG number?
> > 
> > --D
> 
> I’ll reduce the type field to 3 bits (8 values).
> 
> For the AG number, I can drop the artificial 24-bit cap by packing into 
> an unsigned long and storing it via xa_mk_value(), which provides ~60 
> bits on 64-bit systems and ~28 bits on 32-bit systems.

<nod>

> > 
> >> Signed-off-by: Kundan Kumar <kundan.kumar@samsung.com>
> >> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> >> ---
> >>   fs/xfs/xfs_iomap.h | 31 +++++++++++++++++++++++++++++++
> >>   1 file changed, 31 insertions(+)
> >>
> >> diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
> >> index ebcce7d49446..eaf4513f6759 100644
> >> --- a/fs/xfs/xfs_iomap.h
> >> +++ b/fs/xfs/xfs_iomap.h
> >> @@ -12,6 +12,37 @@ struct xfs_inode;
> >>   struct xfs_bmbt_irec;
> >>   struct xfs_zone_alloc_ctx;
> >>   
> >> +/* pack prediction in a u32 stored in xarray */
> >> +#define XFS_AGP_VALID_SHIFT 31
> >> +#define XFS_AGP_TYPE_SHIFT 24
> >> +#define XFS_AGP_TYPE_MASK 0x7fu
> >> +#define XFS_AGP_AGNO_MASK 0x00ffffffu
> >> +
> >> +static inline u32 xfs_agp_pack(u32 agno, u8 iomap_type, bool valid)
> >> +{
> >> +	u32 v = agno & XFS_AGP_AGNO_MASK;
> >> +
> >> +	v |= ((u32)iomap_type & XFS_AGP_TYPE_MASK) << XFS_AGP_TYPE_SHIFT;
> >> +	if (valid)
> >> +		v |= (1u << XFS_AGP_VALID_SHIFT);
> >> +	return v;
> >> +}
> >> +
> >> +static inline bool xfs_agp_valid(u32 v)
> >> +{
> >> +	return v >> XFS_AGP_VALID_SHIFT;

Isn't this just a mask?

	return v & (1U << XFS_AGP_VALID_SHIFT)

> >> +}
> >> +
> >> +static inline u32 xfs_agp_agno(u32 v)
> >> +{
> >> +	return v & XFS_AGP_AGNO_MASK;
> >> +}
> >> +
> >> +static inline u8 xfs_agp_type(u32 v)
> >> +{
> >> +	return (u8)((v >> XFS_AGP_TYPE_SHIFT) & XFS_AGP_TYPE_MASK);
> >> +}

And as Nirjhar noted, please try to use richer types when possible.
s/u32 agno/xfs_agnumber_t agno/

--D

> >> +
> >>   int xfs_iomap_write_direct(struct xfs_inode *ip, xfs_fileoff_t offset_fsb,
> >>   		xfs_fileoff_t count_fsb, unsigned int flags,
> >>   		struct xfs_bmbt_irec *imap, u64 *sequence);
> >> -- 
> >> 2.25.1
> >>
> >>
> > 
> 
>