On Fri, Jun 30, 2017 at 01:04:45PM +0200, Oleg Nesterov wrote: > On 06/29, Paul E. McKenney wrote: > > > > --- a/kernel/task_work.c > > +++ b/kernel/task_work.c > > @@ -109,7 +109,8 @@ void task_work_run(void) > > * the first entry == work, cmpxchg(task_works) should > > * fail, but it can play with *work and other entries. > > */ > > - raw_spin_unlock_wait(&task->pi_lock); > > + raw_spin_lock(&task->pi_lock); > > + raw_spin_unlock(&task->pi_lock); > > Well, bit the you need spin_lock_irq(). And this is one of the reasons > why I personally think unlock_wait have some sense... Good catch, and I clearly need to double-check the other commits for any need to disable interrupts. Anyway, like this, with the addition of a flags variable, correct? > > + raw_spin_lock_irq(&task->pi_lock); > > + raw_spin_unlock_irq(&task->pi_lock); I agree that the spin_unlock_wait() implementations would avoid the deadlock with an acquisition from an interrupt handler, while also avoiding the need to momentarily disable interrupts. The ->pi_lock is a per-task lock, so I am assuming (perhaps naively) that contention is not a problem. So is the overhead of interrupt disabling likely to be noticeable here? Thanx, Paul
