xfs: stop the sync worker before xfs_unmountfs

Ben Myers bpm at sgi.com
Thu Sep 13 11:43:44 CDT 2012


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 at sgi.com>
> >
> > 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



More information about the xfs mailing list