Re: [PATCH 04/21] sched: Change the ttwu success details

[Tejun Heo <tj@kernel.org>]

Hello, Peter.

On Wed, Apr 13, 2011 at 01:06:39PM +0200, Peter Zijlstra wrote:
> On Wed, 2011-04-13 at 12:48 +0200, Peter Zijlstra wrote:
> > Appears to be sufficient to cause the lockup, so somehow the whole
> > workqueue stuff relies on the fact that waking a TASK_(UN)INTERRUPTIBLE
> > task that hasn't been dequeued yet isn't a wakeup.
> > 
> > Tejun any quick clues as to why and how to cure this?
> > 
> > /me goes read that stuff
> 
> OK, so wq_worker_waking_up() does an atomic_inc() that wants to be
> balanced against the atomic_dec() in wq_worker_sleeping(), which is only
> called when we dequeue things.

Yeap, the root cause of the problem is that the change makes
wq_worker_sleeping() and wq_worker_waking_up() asymmetric and thus
puts the nr_running counter goes out of sync which hides active worker
depletion from the workqueue code leading to stall.

One way to deal with it would be adding an extra worker flag to track
sleep state from workqueue side so that it can filter out spurious
wakeups; however, I think it would be far better to resolve this from
scheduler side.  If the callback name is misleading, rename it to
wq_worker_sched_activated() or something and call it only when the
task gets activated.

Thanks.

-- 
tejun