xfs
[Top] [All Lists]

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

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH 5/5] xfs: fix xfs_quiesce_data
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 11 May 2009 16:15:11 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <4A071F06.8040106@xxxxxxxxxxx>
References: <20090426140305.113371000@xxxxxxxxxxxxxxxxxxxxxx> <20090426140708.279368000@xxxxxxxxxxxxxxxxxxxxxx> <4A071F06.8040106@xxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Sun, May 10, 2009 at 01:37:58PM -0500, Eric Sandeen wrote:
> 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?

Ok.

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

Hehe, maybe Dave can chime in.  Or I can test if it actually affects
anything and maybe move it out to another patch.  The lack of
reliability of the filesystreams tests doesn't make this any easier to
test.

> 
> 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
> > 
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---

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