xfs
[Top] [All Lists]

Re: Code Changes [Review Request]: xfs_trans_ail_delete() gets invoked w

To: Amit Sahrawat <amit.sahrawat83@xxxxxxxxx>
Subject: Re: Code Changes [Review Request]: xfs_trans_ail_delete() gets invoked with corrupted values ? Causing Crash and Deadlock.
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 13 Sep 2011 11:27:06 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <CADDb1s0HwQtno3qWJd5CDriGD+m1EJ3fKihrRXuGmqayzwb_Pw@xxxxxxxxxxxxxx>
References: <CADDb1s0HwQtno3qWJd5CDriGD+m1EJ3fKihrRXuGmqayzwb_Pw@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
Hmm, I don't think this is the correct fix.  We should never have
buffers still around once we unmount the log.  Can you try the patch
below?

---
From: Christoph Hellwig <hch@xxxxxx>
Subject: xfs: fix buffer flushing during unmount

The code to flush buffers in the umount code is a bit iffy: we first flush
all delwri buffers out, but then might be able to queue up a new one when
logging the sb counts.  On a normal shutdown that one would get flushed
out when doing the synchronous superblock write in xfs_unmountfs_writesb,
but we skip that one if the filesystem has been shut down.

Fix this by moving the delwri list flushing until just before unmounting
the log, and while we're at it also remove the superflous delwri list
and buffer lru flusing for the rt and log device that can never have
cached or delwri buffers.

Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Index: xfs/fs/xfs/xfs_mount.c
===================================================================
--- xfs.orig/fs/xfs/xfs_mount.c 2011-09-13 10:55:20.748087866 -0400
+++ xfs/fs/xfs/xfs_mount.c      2011-09-13 10:56:19.108088343 -0400
@@ -44,9 +44,6 @@
 #include "xfs_trace.h"
 
 
-STATIC void    xfs_unmountfs_wait(xfs_mount_t *);
-
-
 #ifdef HAVE_PERCPU_SB
 STATIC void    xfs_icsb_balance_counter(xfs_mount_t *, xfs_sb_field_t,
                                                int);
@@ -1496,11 +1493,6 @@ xfs_unmountfs(
         */
        xfs_log_force(mp, XFS_LOG_SYNC);
 
-       xfs_binval(mp->m_ddev_targp);
-       if (mp->m_rtdev_targp) {
-               xfs_binval(mp->m_rtdev_targp);
-       }
-
        /*
         * Unreserve any blocks we have so that when we unmount we don't account
         * the reserved free space as used. This is really only necessary for
@@ -1526,7 +1518,16 @@ xfs_unmountfs(
                xfs_warn(mp, "Unable to update superblock counters. "
                                "Freespace may not be correct on next mount.");
        xfs_unmountfs_writesb(mp);
-       xfs_unmountfs_wait(mp);                 /* wait for async bufs */
+
+       /*
+        * Make sure all buffers have been flushed and completed before
+        * unmounting the log.
+        */
+       error = xfs_flush_buftarg(mp->m_ddev_targp, 1);
+       if (error)
+               xfs_warn(mp, "%d busy buffers during unmount.", error);
+       xfs_wait_buftarg(mp->m_ddev_targp);
+
        xfs_log_unmount_write(mp);
        xfs_log_unmount(mp);
        xfs_uuid_unmount(mp);
@@ -1537,16 +1538,6 @@ xfs_unmountfs(
        xfs_free_perag(mp);
 }
 
-STATIC void
-xfs_unmountfs_wait(xfs_mount_t *mp)
-{
-       if (mp->m_logdev_targp != mp->m_ddev_targp)
-               xfs_wait_buftarg(mp->m_logdev_targp);
-       if (mp->m_rtdev_targp)
-               xfs_wait_buftarg(mp->m_rtdev_targp);
-       xfs_wait_buftarg(mp->m_ddev_targp);
-}
-
 int
 xfs_fs_writable(xfs_mount_t *mp)
 {

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