All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Jackson <pj@sgi.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Gautham R Shenoy <ego@in.ibm.com>
Cc: kosaki.motohiro@jp.fujitsu.com, vegard.nossum@gmail.com,
	menage@google.com, containers@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, maxk@qualcomm.com
Subject: Re: v2.6.26-rc7/cgroups: circular locking dependency
Date: Mon, 23 Jun 2008 07:02:23 -0500	[thread overview]
Message-ID: <20080623070223.eaa8e130.pj@sgi.com> (raw)
In-Reply-To: <1214149823.3223.313.camel@lappy.programming.kicks-ass.net>

CC'd Gautham R Shenoy <ego@in.ibm.com>.

I believe that we had the locking relation between what had been
cgroup_lock (global cgroup lock which can be held over large stretches
of non-performance critical code) and callback_mutex (global cpuset
specific lock which is held over shorter stretches of more performance
critical code - though still not on really hot code paths.)  One can
nest callback_mutex inside cgroup_lock, but not vice versa.

The callback_mutex guarded some CPU masks and Node masks, which might
be multi-word and hence don't change atomically.  Any low level code
that needs to read these these cpuset CPU and Node masks, needs to
hold callback_mutex briefly, to keep that mask from changing while
being read.

There is even a comment in kernel/cpuset.c, explaining how an ABBA
deadlock must be avoided when calling rebuild_sched_domains():

/*
 * rebuild_sched_domains()
 *
 * ...
 *
 * Call with cgroup_mutex held.  May take callback_mutex during
 * call due to the kfifo_alloc() and kmalloc() calls.  May nest
 * a call to the get_online_cpus()/put_online_cpus() pair.
 * Must not be called holding callback_mutex, because we must not
 * call get_online_cpus() while holding callback_mutex.  Elsewhere
 * the kernel nests callback_mutex inside get_online_cpus() calls.
 * So the reverse nesting would risk an ABBA deadlock.

This went into the kernel sometime around 2.6.18.

Then in October and November of 2007, Gautham R Shenoy submitted
"Refcount Based Cpu Hotplug" (http://lkml.org/lkml/2007/11/15/239)

This added cpu_hotplug.lock, which at first glance seems to fit into
the locking hierarchy about where callback_mutex did before, such as
being invocable from rebuild_sched_domains().

However ... the kernel/cpuset.c comments were not updated to describe
the intended locking hierarchy as it relates to cpu_hotplug.lock, and
it looks as if cpu_hotplug.lock can also be taken while invoking the
hotplug callbacks, such as the one here that is handling a CPU down
event for cpusets.

Gautham ... you there?

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

  parent reply	other threads:[~2008-06-23 12:03 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-21 17:38 v2.6.26-rc7/cgroups: circular locking dependency Vegard Nossum
2008-06-22  9:10 ` Cyrill Gorcunov
2008-06-22  9:42   ` Vegard Nossum
     [not found]     ` <19f34abd0806220242p62b9a044la6dd8843e5055f84-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-06-22  9:50       ` Cyrill Gorcunov
2008-06-22  9:50     ` Cyrill Gorcunov
2008-06-22  9:42   ` Vegard Nossum
2008-06-22 15:34 ` KOSAKI Motohiro
2008-06-22 15:50   ` Peter Zijlstra
     [not found]     ` <1214149823.3223.313.camel-N9Ixq/CQ5vKdTX5a5knrm8zTDFooKrT+cvkQGrU6aU0@public.gmane.org>
2008-06-23 12:02       ` Paul Jackson
2008-06-24  6:29       ` Max Krasnyansky
2008-06-23 12:02     ` Paul Jackson [this message]
2008-06-24  6:29     ` Max Krasnyansky
2008-06-26  7:25       ` Paul Menage
2008-06-26 17:45         ` Max Krasnyansky
     [not found]         ` <6599ad830806260025u27458a47j47175ad778ca48a9-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-06-26 17:45           ` Max Krasnyansky
     [not found]       ` <48609441.2030901-zC7DfRvBq/JWk0Htik3J/w@public.gmane.org>
2008-06-26  7:25         ` Paul Menage
     [not found]   ` <2f11576a0806220834m3572ee80i72229cb9a1613558-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-06-22 15:50     ` Peter Zijlstra
2008-06-22 16:02     ` Cyrill Gorcunov
2008-06-22 16:02   ` Cyrill Gorcunov
     [not found] ` <20080621173859.GA6846-nxaqs94rzLmmVpTN6tjYhgHPHiC6WJ74@public.gmane.org>
2008-06-22  9:10   ` Cyrill Gorcunov
2008-06-22 15:34   ` KOSAKI Motohiro
  -- strict thread matches above, loose matches on Subject: below --
2008-06-21 17:38 Vegard Nossum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080623070223.eaa8e130.pj@sgi.com \
    --to=pj@sgi.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=containers@lists.linux-foundation.org \
    --cc=ego@in.ibm.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxk@qualcomm.com \
    --cc=menage@google.com \
    --cc=vegard.nossum@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.