Re: [PATCH 6/7] xfs: make the btree cursor union members named structure

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Mar 12, 2020 at 06:49:29AM -0400, Brian Foster wrote:
> On Wed, Mar 11, 2020 at 08:45:20PM -0700, Darrick J. Wong wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > we need to name the btree cursor private structures to be able
> > to pull them out of the deeply nested structure definition they are
> > in now.
> > 
> > Based on code extracted from a patchset by Darrick Wong.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_btree.h |   36 +++++++++++++++++++++---------------
> >  1 file changed, 21 insertions(+), 15 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
> > index 12a2bc93371d..9884f543eb51 100644
> > --- a/fs/xfs/libxfs/xfs_btree.h
> > +++ b/fs/xfs/libxfs/xfs_btree.h
> > @@ -188,6 +188,24 @@ union xfs_btree_cur_private {
> >  	} abt;
> >  };
> >  
> > +/* Per-AG btree information. */
> > +struct xfs_btree_cur_ag {
> > +	struct xfs_buf			*agbp;
> > +	xfs_agnumber_t			agno;
> > +	union xfs_btree_cur_private	priv;
> > +};
> > +
> > +/* Btree-in-inode cursor information */
> > +struct xfs_btree_cur_ino {
> > +	struct xfs_inode	*ip;
> > +	int			allocated;
> > +	short			forksize;
> > +	char			whichfork;
> > +	char			flags;
> > +#define	XFS_BTCUR_BMBT_WASDEL	(1 << 0)
> > +#define	XFS_BTCUR_BMBT_INVALID_OWNER	(1 << 1)
> > +};
> > +
> 
> Are all of the per-field comments dropped intentionally? These are
> mostly self-explanatory, so either way:

I think the comments were not that valuable, and that I can perhaps
improve them:

/* We are converting a delalloc reservation */
#define	XFS_BTCUR_BMBT_WASDEL		(1 << 0)

/* For extent swap, ignore owner check in verifier */
#define	XFS_BTCUR_BMBT_INVALID_OWNER	(1 << 1)

Anyway, thanks for reviewing this series.

--D

> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
> >  /*
> >   * Btree cursor structure.
> >   * This collects all information needed by the btree code in one place.
> > @@ -209,21 +227,9 @@ typedef struct xfs_btree_cur
> >  	xfs_btnum_t	bc_btnum;	/* identifies which btree type */
> >  	int		bc_statoff;	/* offset of btre stats array */
> >  	union {
> > -		struct {			/* needed for BNO, CNT, INO */
> > -			struct xfs_buf	*agbp;	/* agf/agi buffer pointer */
> > -			xfs_agnumber_t	agno;	/* ag number */
> > -			union xfs_btree_cur_private	priv;
> > -		} bc_ag;
> > -		struct {			/* needed for BMAP */
> > -			struct xfs_inode *ip;	/* pointer to our inode */
> > -			int		allocated;	/* count of alloced */
> > -			short		forksize;	/* fork's inode space */
> > -			char		whichfork;	/* data or attr fork */
> > -			char		flags;		/* flags */
> > -#define	XFS_BTCUR_BMBT_WASDEL	(1 << 0)		/* was delayed */
> > -#define	XFS_BTCUR_BMBT_INVALID_OWNER	(1 << 1)		/* for ext swap */
> > -		} bc_ino;
> > -	};				/* per-btree type data */
> > +		struct xfs_btree_cur_ag	bc_ag;
> > +		struct xfs_btree_cur_ino bc_ino;
> > +	};
> >  } xfs_btree_cur_t;
> >  
> >  /* cursor flags */
> > 
>