| Message ID | 158398472029.1307855.3111787514328025615.stgit@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: make btree cursor private unions anonymous | expand |
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: 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 */ >
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 */ > > >
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: Yeah, it was intentional, because the comments were redundant and the meaning is clear from both the name and the use of the variable. Comments don't always add value... Cheers, Dave.
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) +}; + /* * 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 */