xfs
[Top] [All Lists]

Re: [PATCH 03/76] libxfs: refactor the btree size calculator code

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 03/76] libxfs: refactor the btree size calculator code
From: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
Date: Mon, 4 Jan 2016 14:06:13 -0800
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20151220203928.GS26718@dastard>
References: <20151219085622.12713.88678.stgit@xxxxxxxxxxxxxxxx> <20151219085642.12713.80467.stgit@xxxxxxxxxxxxxxxx> <20151220203928.GS26718@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Dec 21, 2015 at 07:39:28AM +1100, Dave Chinner wrote:
> On Sat, Dec 19, 2015 at 12:56:42AM -0800, Darrick J. Wong wrote:
> > Create a macro to generate btree height calculator functions.
> > This will be used (much) later when we get to the refcount
> > btree.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ....
> > +/* btree size calculator templates */
> > +#define DECLARE_BTREE_SIZE_FN(btree) \
> > +xfs_filblks_t xfs_##btree##_calc_btree_size(struct xfs_mount *mp, \
> > +           unsigned long len);
> > +
> > +#define DEFINE_BTREE_SIZE_FN(btree, limitfield, maxlevels) \
> > +xfs_filblks_t \
> > +xfs_##btree##_calc_btree_size( \
> > +   struct xfs_mount        *mp, \
> > +   unsigned long           len) \
> > +{ \
> > +   int                     level; \
> > +   int                     maxrecs; \
> > +   xfs_filblks_t           rval; \
> > +\
> > +   maxrecs = mp->limitfield[0]; \
> > +   for (level = 0, rval = 0; level < maxlevels; level++) { \
> > +           len += maxrecs - 1; \
> > +           do_div(len, maxrecs); \
> > +           rval += len; \
> > +           if (len == 1) \
> > +                   return rval + maxlevels - \
> > +                           level - 1; \
> > +           if (level == 0) \
> > +                   maxrecs = mp->limitfield[1]; \
> > +   } \
> > +   return rval; \
> > +}
> 
> I really don't like using macros like this. The code becomes hard to
> debug, hard to edit, the functions don't show up in grep/cscope,
> etc.
> 
> A helper function like this:
> 
> xfs_filblks_t
> xfs_btree_calc_size(
>       struct xfs_mount        *mp,
>       int                     *limits,
>       int                     maxlevels,
>       unsigned long           len)
> {
>       int                     level;
>       int                     maxrecs;
>       xfs_filblks_t           rval;
> 
>       maxrecs = limits[0];
>       for (level = 0, rval = 0; level < maxlevels; level++) {
>               len += maxrecs - 1;
>               do_div(len, maxrecs);
>               rval += len;
>               if (len == 1)
>                       return rval + maxlevels - level - 1;
>               if (level == 0)
>                       maxrecs = limits[1];
>       }
>       return rval;
> }
> 
> will work just as well when wrapped with the btree specific calling
> function and that will have none of the problems using a macro to
> build the functions has...

Will change to use a helper function.

--D

> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@xxxxxxxxxxxxx
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH 03/76] libxfs: refactor the btree size calculator code, Darrick J. Wong <=