xfs
[Top] [All Lists]

Re: [PATCH 2/5] libxfs: refactor the btree size calculator code

To: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
Subject: Re: [PATCH 2/5] libxfs: refactor the btree size calculator code
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 1 Feb 2016 10:17:56 -0500
Cc: david@xxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20160123003508.2475.53723.stgit@xxxxxxxxxxxxxxxx>
References: <20160123003502.2475.99558.stgit@xxxxxxxxxxxxxxxx> <20160123003508.2475.53723.stgit@xxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.24 (2015-08-30)
On Fri, Jan 22, 2016 at 04:35:08PM -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>
> ---

The refactoring looks fine to me... though shouldn't this head into the
kernel first and get backported to xfsprogs (or did I miss that
somewhere)?

That and one question below...

>  libxfs/xfs_bmap.c       |   18 +-----------------
>  libxfs/xfs_bmap_btree.c |    9 +++++++++
>  libxfs/xfs_bmap_btree.h |    3 +++
>  libxfs/xfs_btree.c      |   28 ++++++++++++++++++++++++++++
>  libxfs/xfs_btree.h      |    3 +++
>  5 files changed, 44 insertions(+), 17 deletions(-)
> 
> 
> diff --git a/libxfs/xfs_bmap.c b/libxfs/xfs_bmap.c
> index aef7cf3..c134765 100644
> --- a/libxfs/xfs_bmap.c
> +++ b/libxfs/xfs_bmap.c
> @@ -172,25 +172,9 @@ xfs_bmap_worst_indlen(
>       xfs_inode_t     *ip,            /* incore inode pointer */
>       xfs_filblks_t   len)            /* delayed extent length */
>  {
> -     int             level;          /* btree level number */
> -     int             maxrecs;        /* maximum record count at this level */
> -     xfs_mount_t     *mp;            /* mount structure */
>       xfs_filblks_t   rval;           /* return value */
>  
> -     mp = ip->i_mount;
> -     maxrecs = mp->m_bmap_dmxr[0];
> -     for (level = 0, rval = 0;
> -          level < XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK);
> -          level++) {
> -             len += maxrecs - 1;
> -             do_div(len, maxrecs);
> -             rval += len;
> -             if (len == 1)
> -                     return rval + XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) -
> -                             level - 1;
> -             if (level == 0)
> -                     maxrecs = mp->m_bmap_dmxr[1];
> -     }
> +     rval = xfs_bmbt_calc_size(ip->i_mount, len);

We go from xfs_filblks_t to unsigned long here. Isn't the latter 4 bytes
on 32-bit systems? Was that intentional?

Brian

>       return rval;
>  }
>  
> diff --git a/libxfs/xfs_bmap_btree.c b/libxfs/xfs_bmap_btree.c
> index 2ef1836..3c595e2 100644
> --- a/libxfs/xfs_bmap_btree.c
> +++ b/libxfs/xfs_bmap_btree.c
> @@ -880,3 +880,12 @@ xfs_bmbt_change_owner(
>       xfs_btree_del_cursor(cur, error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
>       return error;
>  }
> +
> +xfs_filblks_t
> +xfs_bmbt_calc_size(
> +     struct xfs_mount        *mp,
> +     unsigned long           len)
> +{
> +     return xfs_btree_calc_size(mp, mp->m_bmap_dmxr,
> +                     XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK), len);
> +}
> diff --git a/libxfs/xfs_bmap_btree.h b/libxfs/xfs_bmap_btree.h
> index 819a8a4..04bc6e2 100644
> --- a/libxfs/xfs_bmap_btree.h
> +++ b/libxfs/xfs_bmap_btree.h
> @@ -140,4 +140,7 @@ extern int xfs_bmbt_change_owner(struct xfs_trans *tp, 
> struct xfs_inode *ip,
>  extern struct xfs_btree_cur *xfs_bmbt_init_cursor(struct xfs_mount *,
>               struct xfs_trans *, struct xfs_inode *, int);
>  
> +extern xfs_filblks_t xfs_bmbt_calc_size(struct xfs_mount *mp,
> +             unsigned long len);
> +
>  #endif       /* __XFS_BMAP_BTREE_H__ */
> diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
> index 658f27e..d5f16c5 100644
> --- a/libxfs/xfs_btree.c
> +++ b/libxfs/xfs_btree.c
> @@ -4081,6 +4081,34 @@ xfs_btree_change_owner(
>       return 0;
>  }
>  
> +/*
> + * xfs_btree_calc_size() -- Calculate the number of blocks needed to store
> + *                       a given number of records.
> + */
> +xfs_filblks_t
> +xfs_btree_calc_size(
> +     struct xfs_mount        *mp,
> +     uint                    *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;
> +}
> +
>  /**
>   * xfs_btree_sblock_v5hdr_verify() -- verify the v5 fields of a short-format
>   *                                 btree block
> diff --git a/libxfs/xfs_btree.h b/libxfs/xfs_btree.h
> index 48cb251..b25ffd3 100644
> --- a/libxfs/xfs_btree.h
> +++ b/libxfs/xfs_btree.h
> @@ -465,6 +465,9 @@ static inline int xfs_btree_get_level(struct 
> xfs_btree_block *block)
>  #define XFS_BTREE_TRACE_ARGR(c, r)
>  #define      XFS_BTREE_TRACE_CURSOR(c, t)
>  
> +xfs_filblks_t xfs_btree_calc_size(struct xfs_mount *mp, uint *limits,
> +             int maxlevels, unsigned long len);
> +
>  bool xfs_btree_sblock_v5hdr_verify(struct xfs_buf *bp);
>  bool xfs_btree_sblock_verify(struct xfs_buf *bp, unsigned int max_recs);
>  
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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