xfs
[Top] [All Lists]

Re: [PATCH 5/5] xfs: fix xfs_quiesce_data

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 5/5] xfs: fix xfs_quiesce_data
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Sun, 10 May 2009 13:37:58 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20090426140708.279368000@xxxxxxxxxxxxxxxxxxxxxx>
References: <20090426140305.113371000@xxxxxxxxxxxxxxxxxxxxxx> <20090426140708.279368000@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.21 (Macintosh/20090302)
Christoph Hellwig wrote:

> We need to do a synchronous xfs_sync_fsdata to make sure the superblock
> actually is on disk when we return.  While we're at it also remove the
> superflous SYNC_BDFLUSH flag to xfs_sync_inodes, and move the 
> xfs_filestream_flush
> call later [hch: why?  seems unrelated].

What makes SYNC_BDFLUSH "superfluous?" ... oh ... because nothing in
that callchain below ever looks for it.  Could we make that more obvious
in the changelog?

I guess I too would like a reason for the filestream_flush move
somewhere in the changelog...

Adding the SYNC_WAIT looks good to me though :)

-Eric

> 
> Index: linux-2.6/fs/xfs/linux-2.6/xfs_sync.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/linux-2.6/xfs_sync.c        2009-04-26 
> 10:46:17.112949525 +0200
> +++ linux-2.6/fs/xfs/linux-2.6/xfs_sync.c     2009-04-26 10:48:19.713979813 
> +0200
> @@ -323,16 +323,16 @@ xfs_quiesce_data(
>       int error;
>  
>       /* push non-blocking */
> -     xfs_sync_inodes(mp, SYNC_DELWRI|SYNC_BDFLUSH);
> +     xfs_sync_inodes(mp, SYNC_DELWRI);
>       XFS_QM_DQSYNC(mp, SYNC_BDFLUSH);
> -     xfs_filestream_flush(mp);
>  
> -     /* push and block */
> +     /* push and block till complete */
>       xfs_sync_inodes(mp, SYNC_DELWRI|SYNC_WAIT|SYNC_IOWAIT);
>       XFS_QM_DQSYNC(mp, SYNC_WAIT);
> +     xfs_filestream_flush(mp);
>  
>       /* write superblock and hoover up shutdown errors */
> -     error = xfs_sync_fsdata(mp, 0);
> +     error = xfs_sync_fsdata(mp, SYNC_WAIT);
>  
>       /* flush data-only devices */
>       if (mp->m_rtdev_targp)
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

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