xfs
[Top] [All Lists]

Re: [PATCH] xfs: shutdown xfs_sync_worker before the log

To: Ben Myers <bpm@xxxxxxx>
Subject: Re: [PATCH] xfs: shutdown xfs_sync_worker before the log
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 20 Jun 2012 03:36:00 -0400
Cc: Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20120525204536.GA4721@xxxxxxx>
References: <20120323174327.GU7762@xxxxxxx> <20120514203449.GE16099@xxxxxxx> <20120516015626.GN25351@dastard> <20120516170402.GD3963@xxxxxxx> <20120517071658.GP25351@dastard> <20120524223952.GU16099@xxxxxxx> <20120525204536.GA4721@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, May 25, 2012 at 03:45:36PM -0500, Ben Myers wrote:
> Hey Dave,
> 
> On Thu, May 24, 2012 at 05:39:52PM -0500, Ben Myers wrote:
> > Anyway, I'll make some time to work on this tomorrow so I can test it over
> > the weekend.
> 
> This is going to spin over the weekend.  See what you think.
> 
> -----------
> 
> xfs:  shutdown xfs_sync_worker before the log
> 
> Revert commit 1307bbd, which uses the s_umount semaphore to provide
> exclusion between xfs_sync_worker and unmount, in favor of shutting down
> the sync worker before freeing the log in xfs_log_unmount.  This is a
> cleaner way of resolving the race between xfs_sync_worker and unmount
> than using s_umount.

I like the cancel_delayed_work_sync on unmount side of this, but I don't
really like the MS_ACTIVE check - why can't we only do the initial
xfs_syncd_queue_sync as the last thing in the mount process?

On just slightly related issue with m_sync_work: what does the force
flush of it from xfs_fs_sync_fs in laptop mode buys us?  The sync
just did the log_force and log coverage, so the only think it will do
is force out the AIL after we did the log force and thus guarantee to
keep the fs busy for a while.  I can't really see how that's actually
useful for batter life.  Note that ->sync_fs is only called for an
epxlicit sync (in addition to umount,remount ro and freeze), so for
normal desktop operation it's entirely irrelevant anyway.

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