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
|