xfs
[Top] [All Lists]

Re: review: use correct buffer flags when reading superblock

To: David Chinner <dgc@xxxxxxx>
Subject: Re: review: use correct buffer flags when reading superblock
From: Lachlan McIlroy <lachlan@xxxxxxx>
Date: Thu, 11 Oct 2007 17:23:45 +1000
Cc: xfs-dev <xfs-dev@xxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <20071011033038.GW995458@sgi.com>
References: <470C8F5B.90705@sgi.com> <20071010112821.GI23367404@sgi.com> <470D8D91.903@sgi.com> <20071011033038.GW995458@sgi.com>
Reply-to: lachlan@xxxxxxx
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.4 (X11/20070604)
David Chinner wrote:
On Thu, Oct 11, 2007 at 12:42:25PM +1000, Lachlan McIlroy wrote:
David Chinner wrote:
On Wed, Oct 10, 2007 at 06:37:47PM +1000, Lachlan McIlroy wrote:
When reading the superblock during log recovery we are not setting
the correct buffer flags.  Specifically we are not turning off flags
we do not need such as XBF_ASYNC that is causing the synchronous
xfs_iowait() to hang.  We should also turn off XBF_WRITE and remove
the buffer from the delay write queue just to be safe.
We really don't need the removal of the write flags - the XFS_bflush()
call above the xfs_getsb() call guarantees that they won't be set....
It's not obvious though.  It wasn't obvious that ASYNC was still set
and look where that got us.

Sure, but it is incorrect to have them set at that point - incorrect enough that it might cause corruption. That is, if they are set then the changes made during log replay have not been written to disk and we've got bigger problems than a hanging I/O to worry about....

So rather than clearing them, we should be asserting that the
write flags are not set at all.


New patch attached.
--- fs/xfs/xfs_log_recover.c_1.329      2007-10-10 15:59:18.000000000 +1000
+++ fs/xfs/xfs_log_recover.c    2007-10-11 15:03:00.000000000 +1000
@@ -3824,7 +3824,10 @@ xlog_do_recover(
         */
        bp = xfs_getsb(log->l_mp, 0);
        XFS_BUF_UNDONE(bp);
+       ASSERT(!(XFS_BUF_ISWRITE(bp)));
+       ASSERT(!(XFS_BUF_ISDELAYWRITE(bp)));
        XFS_BUF_READ(bp);
+       XFS_BUF_UNASYNC(bp);
        xfsbdstrat(log->l_mp, bp);
        if ((error = xfs_iowait(bp))) {
                xfs_ioerror_alert("xlog_do_recover",
<Prev in Thread] Current Thread [Next in Thread>