xfs
[Top] [All Lists]

Re: [xfs-masters] [patch 2/2] fs/xfs_super: replace schedule_timeout() w

To: Nishanth Aravamudan <nacc@xxxxxxxxxx>
Subject: Re: [xfs-masters] [patch 2/2] fs/xfs_super: replace schedule_timeout() with msleep_interruptible()
From: Nathan Scott <nathans@xxxxxxx>
Date: Thu, 10 Mar 2005 11:26:22 +1100
Cc: domen@xxxxxxxxxxxx, linux-xfs@xxxxxxxxxxx
In-reply-to: <20050309174544.GA2739@xxxxxxxxxx>
References: <20050306104000.A71391F204@xxxxxxxxxxxxxxxxxxx> <20050308000114.GG720@frodo> <20050308002031.GG2778@xxxxxxxxxx> <20050309032736.GA1687@frodo> <20050309174544.GA2739@xxxxxxxxxx>
Sender: linux-xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.3i
On Wed, Mar 09, 2005 at 09:45:44AM -0800, Nishanth Aravamudan wrote:
> On Wed, Mar 09, 2005 at 02:27:36PM +1100, Nathan Scott wrote:
> > On Mon, Mar 07, 2005 at 04:20:31PM -0800, Nishanth Aravamudan wrote:
> > > While youre interface may be in human-sensible units, the internal timer
> > > subsystem is not (jiffies only exist in the kernel).
> > > msleep_interruptible() changes this, clearly. My patch doesn't really
> > > change anything. It really shouldn't really result in any different
> > > behavior as far as I can tell. msleep_interruptible() takes a timeout
> > 
> > Actually, discussing with Christoph a bit he's pointed out this is
> > going to cause problems for the cases (both xfsbufd and xfssyncd)
> > where we manually wakeup those processes - and there are several
> > situations where we need to do that.
> 
> Could you explain this further? The existing code doesn't sleep on a
> wait-queue, so msleep_interruptible() should not affect that. If you
> wake-up the processes via a manual signal, then msleep_interruptible()
> will still wake up. I'm not seeing how this patch changes any relevant
> code paths (I'm admittedly not an XFS expert).

Both xfsbufd and xfssyncd can be woken up explictly via calls to
wake_up_process - if I'm reading the msleep/msleep_interruptible
code correctly, we'll not return from there until the timeout
has passed (or a signal is delivered in the interruptible variant)
which is not the intention - if those processes are woken through
the XFS internal wake_up_process calls, they need to wakeup pronto.
See pagebuf_daemon_wakeup and xfs_syncd_queue_work for example.

Also, your xfs_buf.c patch is using msleep, but the xfs_super.c
one isn't - seems a bit odd. (?)

cheers.

-- 
Nathan


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