Linux Container Development
 help / color / mirror / Atom feed
From: Paul Menage <menage@google.com>
To: akpm@linuxfoundation.org, balbir@linux.vnet.ibm.com,
	"Serge E. Hallyn" <serue@us.ibm.com>,
	Cedric Le Goater <clg@fr.ibm.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Pavel
Cc: Nick Piggin <nickpiggin@yahoo.com.au>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	pj@sgi.com, containers@lists.osdl.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 08/33] task containersv11 shared container subsystem group arrays avoid lockdep warning
Date: Mon, 17 Sep 2007 14:03:15 -0700	[thread overview]
Message-ID: <20070917210427.392714000@menage.corp.google.com> (raw)
In-Reply-To: 20070917210307.116234000@menage.corp.google.com

[-- Attachment #1: task-containersv11-shared-container-subsystem-group-arrays-avoid-lockdep-warning.patch --]
[-- Type: text/plain, Size: 2900 bytes --]

I think this is the right way to handle the lockdep false-positive in the
current cgroups patches, but I'm not that familiar with lockdep so any
suggestions for a better approach are welcomed.

In order to avoid a false-positive lockdep warning, we lock the root inode
of a new filesystem mount prior to taking cgroup_mutex, to preserve the
invariant that cgroup_mutex nests inside inode->i_mutex.  In order to
prevent a lockdep false positive when locking i_mutex on a newly-created
cgroup directory inode we use mutex_lock_nested(), with a nesting level
of I_MUTEX_CHILD since the new inode will ultimately be a child directory
of the parent whose i_mutex is nested outside of cgroup_mutex.

Signed-off-by: Paul Menage <menage@google.com>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---

 kernel/cgroup.c |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)

diff -puN kernel/cgroup.c~task-cgroupsv11-shared-cgroup-subsystem-group-arrays-avoid-lockdep-warning kernel/cgroup.c
--- a/kernel/cgroup.c~task-cgroupsv11-shared-cgroup-subsystem-group-arrays-avoid-lockdep-warning
+++ a/kernel/cgroup.c
@@ -867,13 +867,16 @@ static int cgroup_get_sb(struct file_
 	} else {
 		/* New superblock */
 		struct cgroup *cont = &root->top_cgroup;
+		struct inode *inode;
 
 		BUG_ON(sb->s_root != NULL);
 
 		ret = cgroup_get_rootdir(sb);
 		if (ret)
 			goto drop_new_super;
+		inode = sb->s_root->d_inode;
 
+		mutex_lock(&inode->i_mutex);
 		mutex_lock(&cgroup_mutex);
 
 		/*
@@ -886,12 +889,14 @@ static int cgroup_get_sb(struct file_
 		ret = allocate_cg_links(css_set_count, &tmp_cg_links);
 		if (ret) {
 			mutex_unlock(&cgroup_mutex);
+			mutex_unlock(&inode->i_mutex);
 			goto drop_new_super;
 		}
 
 		ret = rebind_subsystems(root, root->subsys_bits);
 		if (ret == -EBUSY) {
 			mutex_unlock(&cgroup_mutex);
+			mutex_unlock(&inode->i_mutex);
 			goto drop_new_super;
 		}
 
@@ -931,16 +936,8 @@ static int cgroup_get_sb(struct file_
 		BUG_ON(!list_empty(&cont->children));
 		BUG_ON(root->number_of_cgroups != 1);
 
-		/*
-		 * I believe that it's safe to nest i_mutex inside
-		 * cgroup_mutex in this case, since no-one else can
-		 * be accessing this directory yet. But we still need
-		 * to teach lockdep that this is the case - currently
-		 * a cgroupfs remount triggers a lockdep warning
-		 */
-		mutex_lock(&cont->dentry->d_inode->i_mutex);
 		cgroup_populate_dir(cont);
-		mutex_unlock(&cont->dentry->d_inode->i_mutex);
+		mutex_unlock(&inode->i_mutex);
 		mutex_unlock(&cgroup_mutex);
 	}
 
@@ -1358,7 +1355,7 @@ static int cgroup_create_file(struct 
 
 		/* start with the directory inode held, so that we can
 		 * populate it without racing with another mkdir */
-		mutex_lock(&inode->i_mutex);
+		mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD);
 	} else if (S_ISREG(mode)) {
 		inode->i_size = 0;
 		inode->i_fop = &cgroup_file_operations;
_

--

  parent reply	other threads:[~2007-09-17 21:03 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-17 21:03 [PATCH 00/33] Rename "Task Containers" to "Control Groups" Paul Menage
2007-09-17 21:03 ` [PATCH 01/33] task containersv11 basic task container framework Paul Menage
2007-09-17 21:03 ` [PATCH 02/33] task containersv11 basic task container framework fix Paul Menage
2007-09-17 21:03 ` [PATCH 03/33] task containersv11 add tasks file interface Paul Menage
2007-10-03  8:09   ` Paul Jackson
2007-10-03 15:16     ` Paul Menage
2007-10-03 17:51       ` Paul Jackson
2007-10-03 18:15         ` Paul Menage
2007-10-04  2:46       ` Paul Jackson
2007-10-04  2:53         ` Paul Menage
2007-10-04  2:55     ` Paul Jackson
2007-09-17 21:03 ` [PATCH 04/33] task containersv11 add fork exit hooks Paul Menage
2007-09-17 21:03 ` [PATCH 05/33] task containersv11 add container_clone interface Paul Menage
2007-09-17 21:03 ` [PATCH 06/33] task containersv11 add procfs interface Paul Menage
2007-09-17 21:03 ` [PATCH 07/33] task containersv11 shared container subsystem group arrays Paul Menage
2007-09-17 21:03 ` Paul Menage [this message]
2007-09-17 21:03 ` [PATCH 09/33] task containersv11 shared container subsystem group arrays include fix Paul Menage
2007-09-17 21:03 ` [PATCH 10/33] task containersv11 automatic userspace notification of idle containers Paul Menage
2007-09-17 21:03 ` [PATCH 11/33] task containersv11 make cpusets a client of containers Paul Menage
2007-10-04  9:53   ` Paul Jackson
2007-10-04 15:16     ` Paul Menage
2007-10-04 17:31       ` Paul Jackson
     [not found]       ` <6599ad830710040816p63108ab1vb8547b9600b9e659-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2007-10-04 17:32         ` Paul Jackson
2007-09-17 21:03 ` [PATCH 12/33] task containersv11 example cpu accounting subsystem Paul Menage
2007-09-17 21:03 ` [PATCH 13/33] task containersv11 simple task container debug info subsystem Paul Menage
2007-09-17 21:03 ` [PATCH 14/33] task-containersv11-basic-task-container-framework-containers-fix-refcount-bug Paul Menage
2007-09-17 21:03 ` [PATCH 15/33] task-containersv11-add-container_clone-interface-cgroups-fix-refcount-bug Paul Menage
2007-09-17 21:03 ` [PATCH 16/33] add containerstats v3 Paul Menage
2007-09-17 21:03 ` [PATCH 17/33] add containerstats v3 fix Paul Menage
2007-09-17 21:03 ` [PATCH 18/33] containers implement namespace tracking subsystem Paul Menage
2007-09-17 21:03 ` [PATCH 19/33] containers implement namespace tracking subsystem fix order of container subsystems in init kconfig Paul Menage
2007-09-17 21:03 ` [PATCH 20/33] memory controller add documentation Paul Menage
     [not found]   ` <20070917210429.361229000-B63HFAS8fGlSzHKm+aFRNNkmqwFzkYv6@public.gmane.org>
2007-09-18 16:53     ` Randy Dunlap
2007-09-17 21:03 ` [PATCH 21/33] memory controller resource counters v7 Paul Menage
2007-09-17 21:03 ` [PATCH 22/33] memory controller resource counters v7 fix Paul Menage
2007-09-17 21:03 ` [PATCH 23/33] memory controller containers setup v7 Paul Menage
2007-09-17 21:03 ` [PATCH 24/33] memory controller accounting " Paul Menage
2007-09-17 21:03 ` [PATCH 25/33] memory controller memory accounting v7 Paul Menage
2007-09-17 21:03 ` [PATCH 26/33] memory controller task migration v7 Paul Menage
2007-09-17 21:03 ` [PATCH 27/33] memory controller add per container lru and reclaim v7 Paul Menage
2007-09-17 21:03 ` [PATCH 28/33] memory controller add per container lru and reclaim v7 fix Paul Menage
2007-09-17 21:03 ` [PATCH 29/33] memory controller oom handling v7 Paul Menage
2007-09-17 21:03 ` [PATCH 30/33] memory controller add switch to control what type of pages to limit v7 Paul Menage
2007-09-17 21:03 ` [PATCH 31/33] memory controller make page_referenced container aware v7 Paul Menage
2007-09-17 21:03 ` [PATCH 32/33] memory-controller-improve-user-interface Paul Menage
2007-09-17 21:03 ` [PATCH 33/33] memory-controller-make-charging-gfp-mask-aware Paul Menage

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=20070917210427.392714000@menage.corp.google.com \
    --to=menage@google.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linuxfoundation.org \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=clg@fr.ibm.com \
    --cc=containers@lists.osdl.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nickpiggin@yahoo.com.au \
    --cc=pj@sgi.com \
    --cc=serue@us.ibm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox