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

Darrick J. Wong
2016-06-20T19:34:51Z

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@oracle.com>
> > ---
>
> 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@oss.sgi.com
> > http://oss.sgi.com/mailman/listinfo/xfs