xfs-masters
[Top] [All Lists]

[xfs-masters] [patch 2/3] xfs: fix unmount race

To: dgc@xxxxxxx
Subject: [xfs-masters] [patch 2/3] xfs: fix unmount race
From: akpm@xxxxxxxxxxxxxxxxxxxx
Date: Thu, 10 May 2007 23:10:43 -0700
Cc: tes@xxxxxxx, xfs-masters@xxxxxxxxxxx, akpm@xxxxxxxxxxxxxxxxxxxx, michal.k.k.piotrowski@xxxxxxxxx
Reply-to: xfs-masters@xxxxxxxxxxx
Sender: xfs-masters-bounce@xxxxxxxxxxx
From: David Chinner <dgc@xxxxxxx>

It looks like the workqueue is being run while the log is being torn down.

On unmount, we write a log record to indicate a clean unmount just before
we tear the log down.  We sleep on the iclog force semaphore waiting for
the log I/O to complete, which is processed by xfslogd and xlog_iodone().

xlog_iodone() calls xlog_state_done_syncing() which processes the I/O
completion and wakes the unmount process sleeping on the semaphore. 
xlog_state_done_syncing() then does a bit more work before returning to
xlog_iodone() where we check the buffer to determine if it was an async I/o
or not.

However, before we get to this, the unmount process has woken and continued
onwards which then frees the iclogs and the buffer.  Hence we have a race
between xlog_iodone() looking at the buffer and the unmount process tearing
the buffer down.

There's a couple of different ways I can see to fix the problem - the first
is to not reference the buffer in xlog_iodone() after running the callbacks
that may trigger it being freed.  I'd prfer to see if this fixes the
problem before having to do more invasive surgery.

Cc: Michal Piotrowski <michal.k.k.piotrowski@xxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---

 fs/xfs/xfs_log.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff -puN fs/xfs/xfs_log.c~xfs-fix-unmount-race fs/xfs/xfs_log.c
--- a/fs/xfs/xfs_log.c~xfs-fix-unmount-race
+++ a/fs/xfs/xfs_log.c
@@ -967,14 +967,11 @@ xlog_iodone(xfs_buf_t *bp)
        } else if (iclog->ic_state & XLOG_STATE_IOERROR) {
                aborted = XFS_LI_ABORTED;
        }
+       /* log I/O is always issued ASYNC, so we should see that here */
+       WARN_ON(!(XFS_BUF_ISASYNC(bp)));
        xlog_state_done_syncing(iclog, aborted);
-       if (!(XFS_BUF_ISASYNC(bp))) {
-               /*
-                * Corresponding psema() will be done in bwrite().  If we don't
-                * vsema() here, panic.
-                */
-               XFS_BUF_V_IODONESEMA(bp);
-       }
+       /* do not reference bp here - it may have been freed during unmount */
+
 }      /* xlog_iodone */
 
 /*
_


<Prev in Thread] Current Thread [Next in Thread>
  • [xfs-masters] [patch 2/3] xfs: fix unmount race, akpm <=