Hi Dave, On Thu, Sep 13, 2012 at 09:14:06AM +1000, Dave Chinner wrote: > On Wed, Sep 12, 2012 at 01:33:47PM -0500, Ben Myers wrote: > > See what you think of this. Not heavily tested yet, and not pretty... but it > > is fairly minimal. > .... > > Signed-off-by: Ben Myers <bpm@xxxxxxx> > > > > Index: xfs/fs/xfs/xfs_super.c > > =================================================================== > > --- xfs.orig/fs/xfs/xfs_super.c > > +++ xfs/fs/xfs/xfs_super.c > > @@ -919,6 +919,7 @@ xfs_fs_put_super( > > struct xfs_mount *mp = XFS_M(sb); > > > > xfs_filestream_unmount(mp); > > + cancel_delayed_work_sync(&mp->m_sync_work); > > xfs_unmountfs(mp); > > xfs_syncd_stop(mp); > > xfs_freesb(mp); > > This is the only hunk in the patch needed to fix the problem. > > The rest of the patch does not change the order in which the sync > worker is started and stopped - it just open codes it next to the > xfs_syncd_start/stop calls. Essentially, all it does is obfuscate > the real fix that is being made here and makes it harder to > understand what the relationship between xfs_sync_worker() and > xfs_syncd_start/stop is supposed to be. > > So a minimal patch, IMO, is just the above hunk.... Ah, you're right. I was working on the assumption that it is best not to cancel the work twice. There really is no harm in cancel_delayed_work_sync both in xfs_fs_put_super and in xfs_syncd_stop. However, we don't want them spread around willy nilly. That can become obfuscatory too. I suggest we should remove the cancel_delayed_work_sync(&mp->m_sync_work) in xfs_log_unmount, leftover from commit 11159a05. It seems like that one hasn't been effective. Maybe we don't want to do that in this patch... I'll just add a comment and repost. Thanks, Ben _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
