Re: [PATCH 3.14-rt] sched/numa: Fix task_numa_free() lockdep splat

From: Peter Zijlstra
Date: Tue May 27 2014 - 14:26:21 EST


On Tue, May 27, 2014 at 02:18:36PM -0400, Steven Rostedt wrote:
> [ moving this to LKML from linux-rt-users, as that's where it should be ]
>
> On Sat, 17 May 2014 05:36:59 +0200
> Mike Galbraith <umgwanakikbuti@xxxxxxxxx> wrote:
>
> > 3.14-rt being build with a non-rt config is unlikely, but..
> >
> > >From 60e69eed85bb7b5198ef70643b5895c26ad76ef7 Mon Sep 17 00:00:00 2001
> > From: Mike Galbraith <bitbucket@xxxxxxxxx>
> > Date: Mon, 7 Apr 2014 10:55:15 +0200
> > Subject: [PATCH] sched/numa: Fix task_numa_free() lockdep splat
> >
> > Sasha reported that lockdep claims that the following commit:
> > made numa_group.lock interrupt unsafe:
> >
> > 156654f491dd ("sched/numa: Move task_numa_free() to __put_task_struct()")
> >
> > While I don't see how that could be, given the commit in question moved
> > task_numa_free() from one irq enabled region to another, the below does
> > make both gripes and lockups upon gripe with numa=fake=4 go away.
>
> It wasn't the irqs that was causing the lockdep splat, but the
> softirqs. You moved it into __put_task_struct() which is called as a
> rcu callback that gets called from soft irqs. So yes, you need to
> prevent softirqs from happening whenever you take the lock.
> spin_lock_irq() is a bigger hammer than needed. The patch below should
> be good enough.
>
> I kept the double_lock_irq() as there is no double_lock_bh(). Should we
> bother to make one?

Nope, its really IRQs.

do_exit()
exit_itimers()
itimer_delete()
spin_lock_irqsave(&timer->it_lock, &flags);
timer_delete_hook(timer);
kc->timer_del(timer) := posix_cpu_timer_del()
put_task_struct()
__put_task_struct()
task_numa_free()
spin_lock(&grp->lock);

Which nests the grp->lock inside the timer->it_lock, and where the
timer->it_lock is IRQ-safe, the grp->lock is not.

This allows for IRQ deadlocks.


Attachment: pgpNN_Br_7SuI.pgp
Description: PGP signature