On Wed, Jun 03, 2015 at 04:04:38PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> At the moment, xfs_alloc_fix_freelist() uses a mix of per-ag based
> access and agf buffer based access to freelist and space usage
> information. However, once the AGF buffer is locked inside this
> function, it is guaranteed that both the in-memory and on-disk
> values are identical. xfs_alloc_fix_freelist() doesn't modify the
> values in the structures directly, so it is a read-only user of the
> infomration, and hence can use the per-ag structure exclusively for
> determining what it should do.
>
> This opens up an avenue for cleaning up a lot of duplicated logic
> whose only difference is the structure it gets the data from, and in
> doing so removes a lot of needless byte swapping overhead when
> fixing up the free list.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> fs/xfs/libxfs/xfs_alloc.c | 69 +++++++++++++++++++++--------------------------
> fs/xfs/libxfs/xfs_alloc.h | 8 ++----
> fs/xfs/libxfs/xfs_bmap.c | 3 ++-
> fs/xfs/xfs_filestream.c | 3 ++-
> 4 files changed, 37 insertions(+), 46 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index bc78ac0..08b45f8 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1857,11 +1857,11 @@ xfs_alloc_compute_maxlevels(
> xfs_extlen_t
> xfs_alloc_longest_free_extent(
> struct xfs_mount *mp,
> - struct xfs_perag *pag)
> + struct xfs_perag *pag,
> + xfs_extlen_t need)
> {
> - xfs_extlen_t need, delta = 0;
> + xfs_extlen_t delta = 0;
>
> - need = XFS_MIN_FREELIST_PAG(pag, mp);
> if (need > pag->pagf_flcount)
> delta = need - pag->pagf_flcount;
>
> @@ -1880,10 +1880,8 @@ xfs_alloc_fix_freelist(
> int flags) /* XFS_ALLOC_FLAG_... */
> {
> xfs_buf_t *agbp; /* agf buffer pointer */
> - xfs_agf_t *agf; /* a.g. freespace structure pointer */
> xfs_buf_t *agflbp;/* agfl buffer pointer */
> xfs_agblock_t bno; /* freelist block */
> - xfs_extlen_t delta; /* new blocks needed in freelist */
> int error; /* error result code */
> xfs_extlen_t longest;/* longest extent in allocation group */
> xfs_mount_t *mp; /* file system mount point structure */
> @@ -1927,7 +1925,7 @@ xfs_alloc_fix_freelist(
> * total blocks, reject it.
> */
> need = XFS_MIN_FREELIST_PAG(pag, mp);
> - longest = xfs_alloc_longest_free_extent(mp, pag);
> + longest = xfs_alloc_longest_free_extent(mp, pag, need);
> if ((args->minlen + args->alignment + args->minalignslop - 1) >
> longest ||
> ((int)(pag->pagf_freeblks + pag->pagf_flcount -
> @@ -1954,25 +1952,16 @@ xfs_alloc_fix_freelist(
> return 0;
> }
> }
> - /*
> - * Figure out how many blocks we should have in the freelist.
> - */
> - agf = XFS_BUF_TO_AGF(agbp);
> - need = XFS_MIN_FREELIST(agf, mp);
> - /*
> - * If there isn't enough total or single-extent, reject it.
> - */
> +
> +
> + /* If there isn't enough total space or single-extent, reject it. */
> + need = XFS_MIN_FREELIST_PAG(pag, mp);
> if (!(flags & XFS_ALLOC_FLAG_FREEING)) {
> - delta = need > be32_to_cpu(agf->agf_flcount) ?
> - (need - be32_to_cpu(agf->agf_flcount)) : 0;
> - longest = be32_to_cpu(agf->agf_longest);
> - longest = (longest > delta) ? (longest - delta) :
> - (be32_to_cpu(agf->agf_flcount) > 0 || longest > 0);
> + longest = xfs_alloc_longest_free_extent(mp, pag, need);
> if ((args->minlen + args->alignment + args->minalignslop - 1) >
> longest ||
> - ((int)(be32_to_cpu(agf->agf_freeblks) +
> - be32_to_cpu(agf->agf_flcount) - need - args->total) <
> - (int)args->minleft)) {
> + ((int)(pag->pagf_freeblks + pag->pagf_flcount -
> + need - args->total) < (int)args->minleft)) {
> xfs_trans_brelse(tp, agbp);
> args->agbp = NULL;
> return 0;
> @@ -1980,21 +1969,25 @@ xfs_alloc_fix_freelist(
> }
> /*
> * Make the freelist shorter if it's too long.
> + *
> + * XXX (dgc): When we have lots of free space, does this buy us
> + * anything other than extra overhead when we need to put more blocks
> + * back on the free list? Maybe we should only do this when space is
> + * getting low or the AGFL is more than half full?
> */
> - while (be32_to_cpu(agf->agf_flcount) > need) {
> - xfs_buf_t *bp;
> + while (pag->pagf_flcount > need) {
> + struct xfs_buf *bp;
>
> error = xfs_alloc_get_freelist(tp, agbp, &bno, 0);
> if (error)
> return error;
> - if ((error = xfs_free_ag_extent(tp, agbp, args->agno, bno, 1, 1)))
> + error = xfs_free_ag_extent(tp, agbp, args->agno, bno, 1, 1);
> + if (error)
> return error;
> bp = xfs_btree_get_bufs(mp, tp, args->agno, bno, 0);
> xfs_trans_binval(tp, bp);
> }
> - /*
> - * Initialize the args structure.
> - */
> +
> memset(&targs, 0, sizeof(targs));
> targs.tp = tp;
> targs.mp = mp;
> @@ -2003,18 +1996,18 @@ xfs_alloc_fix_freelist(
> targs.alignment = targs.minlen = targs.prod = targs.isfl = 1;
> targs.type = XFS_ALLOCTYPE_THIS_AG;
> targs.pag = pag;
> - if ((error = xfs_alloc_read_agfl(mp, tp, targs.agno, &agflbp)))
> + error = xfs_alloc_read_agfl(mp, tp, targs.agno, &agflbp);
> + if (error)
> return error;
> - /*
> - * Make the freelist longer if it's too short.
> - */
> - while (be32_to_cpu(agf->agf_flcount) < need) {
> +
> + /* Make the freelist longer if it's too short. */
> + while (pag->pagf_flcount < need) {
> targs.agbno = 0;
> - targs.maxlen = need - be32_to_cpu(agf->agf_flcount);
> - /*
> - * Allocate as many blocks as possible at once.
> - */
> - if ((error = xfs_alloc_ag_vextent(&targs))) {
> + targs.maxlen = need - pag->pagf_flcount;
> +
> + /* Allocate as many blocks as possible at once. */
> + error = xfs_alloc_ag_vextent(&targs);
> + if (error) {
> xfs_trans_brelse(tp, agflbp);
> return error;
> }
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 29f27b2..a4d3b9a 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -130,12 +130,8 @@ typedef struct xfs_alloc_arg {
> #define XFS_ALLOC_USERDATA 1 /* allocation is for user data*/
> #define XFS_ALLOC_INITIAL_USER_DATA 2 /* special case start of file */
>
> -/*
> - * Find the length of the longest extent in an AG.
> - */
> -xfs_extlen_t
> -xfs_alloc_longest_free_extent(struct xfs_mount *mp,
> - struct xfs_perag *pag);
> +xfs_extlen_t xfs_alloc_longest_free_extent(struct xfs_mount *mp,
> + struct xfs_perag *pag, xfs_extlen_t need);
>
> /*
> * Compute and fill in value of m_ag_maxlevels.
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 5cb3e85..7382cce 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3521,7 +3521,8 @@ xfs_bmap_longest_free_extent(
> }
> }
>
> - longest = xfs_alloc_longest_free_extent(mp, pag);
> + longest = xfs_alloc_longest_free_extent(mp, pag,
> + XFS_MIN_FREELIST_PAG(pag, mp));
> if (*blen < longest)
> *blen = longest;
>
> diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
> index da82f1c..9ac5eaa 100644
> --- a/fs/xfs/xfs_filestream.c
> +++ b/fs/xfs/xfs_filestream.c
> @@ -196,7 +196,8 @@ xfs_filestream_pick_ag(
> goto next_ag;
> }
>
> - longest = xfs_alloc_longest_free_extent(mp, pag);
> + longest = xfs_alloc_longest_free_extent(mp, pag,
> + XFS_MIN_FREELIST_PAG(pag, mp));
> if (((minlen && longest >= minlen) ||
> (!minlen && pag->pagf_freeblks >= minfree)) &&
> (!pag->pagf_metadata || !(flags & XFS_PICK_USERDATA) ||
> --
> 2.0.0
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
