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

[Peter Zijlstra <peterz@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 1961 bytes --]

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@gmail.com> 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@online.de>
> > 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 #2: Type: application/pgp-signature, Size: 836 bytes --]