Re: review: use correct buffer flags when reading superblock

[Lachlan McIlroy <lachlan@sgi.com>]

[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]

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.

[-- Attachment #2: xfs_getsb.diff --]
[-- Type: text/x-patch, Size: 455 bytes --]

--- 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",