xfs
[Top] [All Lists]

[PATCH 1/4] xfs: kill XBF_FS_MANAGED buffers

To: xfs@xxxxxxxxxxx
Subject: [PATCH 1/4] xfs: kill XBF_FS_MANAGED buffers
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 9 Sep 2010 01:12:55 +1000
In-reply-to: <1283958778-28610-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1283958778-28610-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

Filesystem level managed buffers are buffers that have their
lifecycle controlled by the filesystem layer, not the buffer cache.
We currently hash these buffers, which makes cleanup and hash
walking somewhat troublesome. Convert the fs managed buffers to
unhashed buffers obtained by via xfs_buf_get_noaddr(), and remove the
XBF_FS_MANAGED special cases from the buffer cache.

Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
 fs/xfs/linux-2.6/xfs_buf.c |   20 +++-------------
 fs/xfs/linux-2.6/xfs_buf.h |    4 ---
 fs/xfs/xfs_mount.c         |   54 +++++++++++++++++++++++++------------------
 3 files changed, 35 insertions(+), 43 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index 998bfb3..860a23c 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -791,8 +791,6 @@ xfs_buf_rele(
                        atomic_inc(&bp->b_hold);
                        spin_unlock(&hash->bh_lock);
                        (*(bp->b_relse)) (bp);
-               } else if (bp->b_flags & XBF_FS_MANAGED) {
-                       spin_unlock(&hash->bh_lock);
                } else {
                        ASSERT(!(bp->b_flags & (XBF_DELWRI|_XBF_DELWRI_Q)));
                        list_del_init(&bp->b_hash_list);
@@ -1401,26 +1399,16 @@ void
 xfs_wait_buftarg(
        xfs_buftarg_t   *btp)
 {
-       xfs_buf_t       *bp, *n;
        xfs_bufhash_t   *hash;
        uint            i;
 
        for (i = 0; i < (1 << btp->bt_hashshift); i++) {
                hash = &btp->bt_hash[i];
-again:
                spin_lock(&hash->bh_lock);
-               list_for_each_entry_safe(bp, n, &hash->bh_list, b_hash_list) {
-                       ASSERT(btp == bp->b_target);
-                       if (!(bp->b_flags & XBF_FS_MANAGED)) {
-                               spin_unlock(&hash->bh_lock);
-                               /*
-                                * Catch superblock reference count leaks
-                                * immediately
-                                */
-                               BUG_ON(bp->b_bn == 0);
-                               delay(100);
-                               goto again;
-                       }
+               while (!list_empty(&hash->bh_list)) {
+                       spin_unlock(&hash->bh_lock);
+                       delay(100);
+                       spin_lock(&hash->bh_lock);
                }
                spin_unlock(&hash->bh_lock);
        }
diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 2a05614..72215d8 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -51,7 +51,6 @@ typedef enum {
 #define XBF_DONE       (1 << 5) /* all pages in the buffer uptodate */
 #define XBF_DELWRI     (1 << 6) /* buffer has dirty pages */
 #define XBF_STALE      (1 << 7) /* buffer has been staled, do not find it */
-#define XBF_FS_MANAGED (1 << 8) /* filesystem controls freeing memory */
 #define XBF_ORDERED    (1 << 11)/* use ordered writes */
 #define XBF_READ_AHEAD (1 << 12)/* asynchronous read-ahead */
 #define XBF_LOG_BUFFER (1 << 13)/* this is a buffer used for the log */
@@ -104,7 +103,6 @@ typedef unsigned int xfs_buf_flags_t;
        { XBF_DONE,             "DONE" }, \
        { XBF_DELWRI,           "DELWRI" }, \
        { XBF_STALE,            "STALE" }, \
-       { XBF_FS_MANAGED,       "FS_MANAGED" }, \
        { XBF_ORDERED,          "ORDERED" }, \
        { XBF_READ_AHEAD,       "READ_AHEAD" }, \
        { XBF_LOCK,             "LOCK" },       /* should never be set */\
@@ -276,8 +274,6 @@ extern void xfs_buf_terminate(void);
                                        XFS_BUF_DONE(bp);       \
                                } while (0)
 
-#define XFS_BUF_UNMANAGE(bp)   ((bp)->b_flags &= ~XBF_FS_MANAGED)
-
 #define XFS_BUF_DELAYWRITE(bp)         ((bp)->b_flags |= XBF_DELWRI)
 #define XFS_BUF_UNDELAYWRITE(bp)       xfs_buf_delwri_dequeue(bp)
 #define XFS_BUF_ISDELAYWRITE(bp)       ((bp)->b_flags & XBF_DELWRI)
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index aeb9d72..b2009e3 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -639,7 +639,6 @@ int
 xfs_readsb(xfs_mount_t *mp, int flags)
 {
        unsigned int    sector_size;
-       unsigned int    extra_flags;
        xfs_buf_t       *bp;
        int             error;
 
@@ -652,17 +651,29 @@ xfs_readsb(xfs_mount_t *mp, int flags)
         * access to the superblock.
         */
        sector_size = xfs_getsize_buftarg(mp->m_ddev_targp);
-       extra_flags = XBF_LOCK | XBF_FS_MANAGED | XBF_MAPPED;
 
-       bp = xfs_buf_read(mp->m_ddev_targp, XFS_SB_DADDR, BTOBB(sector_size),
-                         extra_flags);
+reread:
+       bp = xfs_buf_get_noaddr(sector_size, mp->m_ddev_targp);
+
        if (!bp || XFS_BUF_ISERROR(bp)) {
-               xfs_fs_mount_cmn_err(flags, "SB read failed");
+               xfs_fs_mount_cmn_err(flags, "SB buffer alloc failed");
                error = bp ? XFS_BUF_GETERROR(bp) : ENOMEM;
                goto fail;
        }
-       ASSERT(XFS_BUF_ISBUSY(bp));
-       ASSERT(XFS_BUF_VALUSEMA(bp) <= 0);
+
+       /* set up the buffer for a read IO */
+       xfs_buf_lock(bp);
+       XFS_BUF_SET_ADDR(bp, XFS_SB_DADDR);
+        XFS_BUF_READ(bp);
+        XFS_BUF_BUSY(bp);
+
+       xfsbdstrat(mp, bp);
+       error = xfs_iowait(bp);
+       if (error || XFS_BUF_ISERROR(bp)) {
+               xfs_fs_mount_cmn_err(flags, "SB read failed");
+               error = error ? error : XFS_BUF_GETERROR(bp);
+               goto fail;
+       }
 
        /*
         * Initialize the mount structure from the superblock.
@@ -692,33 +703,25 @@ xfs_readsb(xfs_mount_t *mp, int flags)
         * re-read the superblock so the buffer is correctly sized.
         */
        if (sector_size < mp->m_sb.sb_sectsize) {
-               XFS_BUF_UNMANAGE(bp);
                xfs_buf_relse(bp);
                sector_size = mp->m_sb.sb_sectsize;
-               bp = xfs_buf_read(mp->m_ddev_targp, XFS_SB_DADDR,
-                                 BTOBB(sector_size), extra_flags);
-               if (!bp || XFS_BUF_ISERROR(bp)) {
-                       xfs_fs_mount_cmn_err(flags, "SB re-read failed");
-                       error = bp ? XFS_BUF_GETERROR(bp) : ENOMEM;
-                       goto fail;
-               }
-               ASSERT(XFS_BUF_ISBUSY(bp));
-               ASSERT(XFS_BUF_VALUSEMA(bp) <= 0);
+               goto reread;
        }
 
        /* Initialize per-cpu counters */
        xfs_icsb_reinit_counters(mp);
 
+       /* grab a reference for caching the buffer */
+        XFS_BUF_HOLD(bp);
        mp->m_sb_bp = bp;
+
        xfs_buf_relse(bp);
        ASSERT(XFS_BUF_VALUSEMA(bp) > 0);
        return 0;
 
- fail:
-       if (bp) {
-               XFS_BUF_UNMANAGE(bp);
+fail:
+       if (bp)
                xfs_buf_relse(bp);
-       }
        return error;
 }
 
@@ -2007,9 +2010,14 @@ xfs_freesb(
         * when we call xfs_buf_relse().
         */
        bp = xfs_getsb(mp, 0);
-       XFS_BUF_UNMANAGE(bp);
-       xfs_buf_relse(bp);
        mp->m_sb_bp = NULL;
+
+       /*
+        * need to release the buffer twice to free it because we hold an extra
+        * reference count on it.
+        */
+       xfs_buf_relse(bp);
+       xfs_buf_relse(bp);
 }
 
 /*
-- 
1.7.1

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