On Thu, Mar 10, 2005 at 11:26:22AM +1100, Nathan Scott wrote:
> 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.
I see, I didn't know about the wake_up_process(). You are right that
msleep*() does not play well with wait-queues. I'm working on creating
some patches that might, though.
> Also, your xfs_buf.c patch is using msleep, but the xfs_super.c
> one isn't - seems a bit odd. (?)
Bug on my part. Both patches should be dropped, though, as you said.
Thanks,
Nish
|