Re: [PATCH 06/23 V2] xfs: convert xfs_iwalk to use perag references

From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 06/23 V2] xfs: convert xfs_iwalk to use perag references
Date: Wed, 2 Jun 2021 10:34:45 +1000	[thread overview]
Message-ID: <20210602003445.GI664593@dread.disaster.area> (raw)
In-Reply-To: <20210601233216.GF26380@locust>

On Tue, Jun 01, 2021 at 04:32:16PM -0700, Darrick J. Wong wrote:
> On Wed, Jun 02, 2021 at 08:00:54AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Rather than manually walking the ags and passing agnunbers around,
> > pass the perag for the AG we are currently working on around in the
> > iwalk structure.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > Reviewed-by: Brian Foster <bfoster@redhat.com>
> > ---
> > V2: convert next_agno -> agno in perag walk macros
> > 
> >  fs/xfs/libxfs/xfs_ag.h | 20 +++++++-----
> >  fs/xfs/xfs_iwalk.c     | 86 +++++++++++++++++++++++++++++++-------------------
> >  2 files changed, 66 insertions(+), 40 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> > index 33783120263c..2e02ec3693c5 100644
> > --- a/fs/xfs/libxfs/xfs_ag.h
> > +++ b/fs/xfs/libxfs/xfs_ag.h
> > @@ -117,19 +117,23 @@ void	xfs_perag_put(struct xfs_perag *pag);
> >  /*
> >   * Perag iteration APIs
> >   */
> > -#define for_each_perag(mp, next_agno, pag) \
> > -	for ((next_agno) = 0, (pag) = xfs_perag_get((mp), 0); \
> > +#define for_each_perag_from(mp, agno, pag) \
> > +	for ((pag) = xfs_perag_get((mp), (agno)); \
> 
> Er... I guess I wasn't clear enough.  I was ok with the "next_agno" name
> for for_each_perag_from to reinforce the idea that the caller is
> required to initialize the variable before using the macro.
> 
> It's the other variants (for_each_perag and for_each_perag_tag) where
> the macro initializes @agno so it's not necessary for the caller to have
> provided any value at all.
> 
> IOWS,
> 
> #define for_each_perag_from(mp, next_agno, pag) \
> 	for ((pag) = xfs_perag_get((mp), (next_agno)); \
> 		(pag) != NULL; \
> 		(next_agno) = (pag)->pag_agno + 1, \
> 		xfs_perag_put(pag), \
> 		(pag) = xfs_perag_get((mp), (next_agno)))
> 
> #define for_each_perag(mp, agno, pag) \
> 	(agno) = 0; \
> 	for_each_perag_from((mp), (agno), (pag))
> 
> #define for_each_perag_tag(mp, agno, pag, tag) \
> 	for ((agno) = 0, (pag) = xfs_perag_get_tag((mp), 0, (tag)); \
> 		(pag) != NULL; \
> 		(agno) = (pag)->pag_agno + 1, \
> 		xfs_perag_put(pag), \
> 		(pag) = xfs_perag_get_tag((mp), (agno), (tag)))

Ok, let me go change that back...

-Dave.
-- 
Dave Chinner
david@fromorbit.com