Re: [PATCH 2/2] xfs: Extend xattr extent counter to 32-bits

[Christoph Hellwig <hch@infradead.org>]

FYI, I have had a series in the works for a while but not quite 
finished yet that moves the in-memory nextents and format fields
into the ifork structure.  I feared this might conflict badly, but
so far this seems relatively harmless.  Note that your patch creates
some not so nice layout in struct xfs_icdinode, so maybe I need to
rush and finish that series ASAP.

> +static inline int32_t XFS_DFORK_NEXTENTS(struct xfs_sb *sbp,
> +					struct xfs_dinode *dip, int whichfork)
> +{
> +	int32_t anextents;
> +
> +	if (whichfork == XFS_DATA_FORK)
> +		return be32_to_cpu((dip)->di_nextents);
> +
> +	anextents = be16_to_cpu((dip)->di_anextents_lo);
> +	if (xfs_sb_version_has_v3inode(sbp))
> +		anextents |= ((u32)(be16_to_cpu((dip)->di_anextents_hi)) << 16);
> +
> +	return anextents;

No need for any of the braces around dip.  Also this funcion really
deserves a proper lower case name now, and probably should be moved out
of line.

>  typedef uint32_t	xfs_extlen_t;	/* extent length in blocks */
>  typedef uint32_t	xfs_agnumber_t;	/* allocation group number */
>  typedef int32_t		xfs_extnum_t;	/* # of extents in a file */
> -typedef int16_t		xfs_aextnum_t;	/* # extents in an attribute fork */
> +typedef int32_t		xfs_aextnum_t;	/* # extents in an attribute fork */

We can just retire xfs_aextnum_t.  It only has 4 uses anyway.

> @@ -327,7 +327,7 @@ xfs_inode_to_log_dinode(
>  	to->di_nblocks = from->di_nblocks;
>  	to->di_extsize = from->di_extsize;
>  	to->di_nextents = from->di_nextents;
> -	to->di_anextents = from->di_anextents;
> +	to->di_anextents_lo = ((u32)(from->di_anextents)) & 0xffff;

No need for any of the casting here.

> @@ -3044,7 +3045,14 @@ xlog_recover_inode_pass2(
>  			goto out_release;
>  		}
>  	}
> -	if (unlikely(ldip->di_nextents + ldip->di_anextents > ldip->di_nblocks)){
> +
> +	nextents = ldip->di_anextents_lo;
> +	if (xfs_sb_version_has_v3inode(&mp->m_sb))
> +		nextents |= ((u32)(ldip->di_anextents_hi) << 16);
> +
> +	nextents += ldip->di_nextents;

Little helpers to get/set the attr extents in the log inode would be nice.


Last but not least:  This seems like a feature flag we could just lazily
set once needed, similar to attr2.