xfs
[Top] [All Lists]

[PATCH 01/20] xfs: xfs_alloc_fix_freelist() can use incore perag structu

To: xfs@xxxxxxxxxxx
Subject: [PATCH 01/20] xfs: xfs_alloc_fix_freelist() can use incore perag structures
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 3 Jun 2015 16:04:38 +1000
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1433311497-10245-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1433311497-10245-1-git-send-email-david@xxxxxxxxxxxxx>
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>
---
 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

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