xfs
[Top] [All Lists]

[PATCH 5/6 V2] xfs: fix buffer shudown reference count mismatch

To: xfs@xxxxxxxxxxx
Subject: [PATCH 5/6 V2] xfs: fix buffer shudown reference count mismatch
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 2 Nov 2012 14:23:12 +1100
In-reply-to: <20121102024351.GU29378@dastard>
References: <1351816724-3000-1-git-send-email-david@xxxxxxxxxxxxx> <1351816724-3000-6-git-send-email-david@xxxxxxxxxxxxx> <20121102024351.GU29378@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
xfs: fix buffer shudown reference count mismatch

From: Dave Chinner <dchinner@xxxxxxxxxx>

When we shut down the filesystem, we have to unpin and free all the
buffers currently active in the CIL. To do this we unpin and remove
them in one operation as a result of a failed iclogbuf write. For
buffers, we do this removal via a simultated IO completion of after
marking the buffer stale.

At the time we do this, we have two references to the buffer - the
active LRU reference and the buf log item.  The LRU reference is
removed by marking the buffer stale, and the active CIL reference is
by the xfs_buf_iodone() callback that is run by
xfs_buf_do_callbacks() during ioend processing (via the bp->b_iodone
callback).

However, ioend processing requires one more reference - that of the
IO that it is completing. We don't have this reference, so we free
the buffer prematurely and use it after it is freed. For buffers
marked with XBF_ASYNC, this leads to assert failures in
xfs_buf_rele() on debug kernels because the b_hold count is zero.

Fix this by making sure we take the necessary IO reference before
starting IO completion processing on the stale buffer, and set the
XBF_ASYNC flag to ensure that IO completion processing removes all
the active references from the buffer to ensure it is fully torn
down.

Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
V2 - add XBF_ASYNC to buffers so last reference is always removed.
     Fixes shutdown assert failures due to references from buffers
     remaining the perag structures at unmount.

 fs/xfs/xfs_buf_item.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index a8d0ed9..becf4a9 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -526,7 +526,25 @@ xfs_buf_item_unpin(
                }
                xfs_buf_relse(bp);
        } else if (freed && remove) {
+               /*
+                * There are currently two references to the buffer - the active
+                * LRU reference and the buf log item. What we are about to do
+                * here - simulate a failed IO completion - requires 3
+                * references.
+                *
+                * The LRU reference is removed by the xfs_buf_stale() call. The
+                * buf item reference is removed by the xfs_buf_iodone()
+                * callback that is run by xfs_buf_do_callbacks() during ioend
+                * processing (via the bp->b_iodone callback), and then finally
+                * the ioend processing will drop the IO reference if the buffer
+                * is marked XBF_ASYNC.
+                *
+                * Hence we need to take an additional reference here so that IO
+                * completion processing doesn't free the buffer prematurely.
+                */
                xfs_buf_lock(bp);
+               xfs_buf_hold(bp);
+               bp->b_flags |= XBF_ASYNC;
                xfs_buf_ioerror(bp, EIO);
                XFS_BUF_UNDONE(bp);
                xfs_buf_stale(bp);

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