xfs
[Top] [All Lists]

Re: [PATCH 010/119] xfs: create a standard btree size calculator code

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH 010/119] xfs: create a standard btree size calculator code
From: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
Date: Mon, 20 Jun 2016 12:34:32 -0700
Cc: david@xxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, vishal.l.verma@xxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20160620143148.GA2465@xxxxxxxxxxxxxxx>
References: <146612627129.12839.3827886950949809165.stgit@xxxxxxxxxxxxxxxx> <146612633632.12839.13314497569643567486.stgit@xxxxxxxxxxxxxxxx> <20160620143148.GA2465@xxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.24 (2015-08-30)
On Mon, Jun 20, 2016 at 10:31:49AM -0400, Brian Foster wrote:
> On Thu, Jun 16, 2016 at 06:18:56PM -0700, Darrick J. Wong wrote:
> > Create a helper to generate AG btree height calculator functions.
> > This will be used (much) later when we get to the refcount btree.
> > 
> > v2: Use a helper function instead of a macro.
> > v3: We can (theoretically) store more than 2^32 records in a btree, so
> >     widen the fields to accept that.
> > v4: Don't modify xfs_bmap_worst_indlen; the purpose of /that/ function
> >     is to estimate the worst-case number of blocks needed for a bmbt
> >     expansion, not to calculate the space required to store nr records.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> 
> I think this one should probably be pushed out to where it is used
> (easier to review with an example imo). I don't see it used anywhere up
> through the rmapbt stuff, anyways...

Oh, heh, you're right.  At one point I was using it for the rmapbt, but
nowadays it's only used for per-AG reservations (reflink+rmap) as you
point out, so it could move.

> 
> >  fs/xfs/libxfs/xfs_btree.c |   27 +++++++++++++++++++++++++++
> >  fs/xfs/libxfs/xfs_btree.h |    3 +++
> >  2 files changed, 30 insertions(+)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> > index 105979d..5eb4e40 100644
> > --- a/fs/xfs/libxfs/xfs_btree.c
> > +++ b/fs/xfs/libxfs/xfs_btree.c
> > @@ -4156,3 +4156,30 @@ xfs_btree_sblock_verify(
> >  
> >     return true;
> >  }
> > +
> > +/*
> > + * Calculate the number of blocks needed to store a given number of records
> > + * in a short-format (per-AG metadata) btree.
> > + */
> > +xfs_extlen_t
> > +xfs_btree_calc_size(
> > +   struct xfs_mount        *mp,
> > +   uint                    *limits,
> > +   unsigned long long      len)
> > +{
> > +   int                     level;
> > +   int                     maxrecs;
> > +   xfs_extlen_t            rval;
> > +
> > +   maxrecs = limits[0];
> > +   for (level = 0, rval = 0; len > 0; level++) {
> 
> len is unsigned, so len > 0 is kind of pointless. Perhaps check len > 1
> and kill the check in the loop?

Yup.  Thank you for pointing that out.

--D

> 
> Brian
> 
> > +           len += maxrecs - 1;
> > +           do_div(len, maxrecs);
> > +           rval += len;
> > +           if (len == 1)
> > +                   return rval;
> > +           if (level == 0)
> > +                   maxrecs = limits[1];
> > +   }
> > +   return rval;
> > +}
> > diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
> > index 9a88839..b330f19 100644
> > --- a/fs/xfs/libxfs/xfs_btree.h
> > +++ b/fs/xfs/libxfs/xfs_btree.h
> > @@ -475,4 +475,7 @@ static inline int xfs_btree_get_level(struct 
> > xfs_btree_block *block)
> >  bool xfs_btree_sblock_v5hdr_verify(struct xfs_buf *bp);
> >  bool xfs_btree_sblock_verify(struct xfs_buf *bp, unsigned int max_recs);
> >  
> > +xfs_extlen_t xfs_btree_calc_size(struct xfs_mount *mp, uint *limits,
> > +           unsigned long long len);
> > +
> >  #endif     /* __XFS_BTREE_H__ */
> > 
> > _______________________________________________
> > xfs mailing list
> > xfs@xxxxxxxxxxx
> > http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>