Skip to contentSkip to conversation

CGroups: Add a per-subsystem hierarchy lock

RFC PATCH In review Paul Menage 2 participants 8 messages Jun 27, 2008 – Jul 12, 2008 cgroup

lore
Share & tools
lore

Paul Menage

author
#0 lore raw

RFC PATCH 1/1 CGroups: Add a per-subsystem hierarchy lock

This patch adds a hierarchy_mutex to the cgroup_subsys object that protects changes to the hierarchy observed by that subsystem. It is taken by the cgroup subsystem for the following operations:

- linking a cgroup into that subsystem's cgroup tree
- unlinking a cgroup from that subsystem's cgroup tree
- moving the subsystem to/from a hierarchy (including across the bind() callback)

Thus if the subsystem holds its own hierarchy_mutex, it can safely traverse its ts own hierarchy.

This patch also changes the cpuset hotplug code to take cpuset_subsys.hierarchy_mutex rather than cgroup_mutex, removing a cyclical locking dependancy between cpu_hotplug.lock and cgroup_mutex.

---

This is just a first cut - a full version of this patch would also make use of the hierarchy_mutex at other places in cpuset.c that use the cgroup hierarchy, but I wanted to see what people thought of the concept. It appears to keep lockdep quiet.

 include/linux/cgroup.h |   10 ++++++++--
 kernel/cgroup.c        |   35 ++++++++++++++++++++++++++++++++++-
 kernel/cpuset.c        |    4 ++--
 3 files changed, 44 insertions(+), 5 deletions(-)
include/linux/cgroup.h +8 -2
Index: lockfix-2.6.26-rc5-mm3/include/linux/cgroup.h
===================================================================
@@ -319,9 +319,15 @@ struct cgroup_subsys {
 #define MAX_CGROUP_TYPE_NAMELEN 32
 	const char *name;
 
-	/* Protected by RCU */
-	struct cgroupfs_root *root;
+	/*
+	 * Protects sibling/children links of cgroups in this
+	 * hierarchy, plus protects which hierarchy (or none) the
+	 * subsystem is a part of (i.e. root/sibling)
+	 */
+	struct mutex hierarchy_mutex;
 
+	/* Protected by RCU, this->hierarchy_mutex and cgroup_lock() */
+	struct cgroupfs_root *root;
 	struct list_head sibling;
 
 	void *private;
kernel/cgroup.c +34 -1
Index: lockfix-2.6.26-rc5-mm3/kernel/cgroup.c
===================================================================
@@ -728,23 +728,26 @@ static int rebind_subsystems(struct cgro
 			BUG_ON(cgrp->subsys[i]);
 			BUG_ON(!dummytop->subsys[i]);
 			BUG_ON(dummytop->subsys[i]->cgroup != dummytop);
+			mutex_lock(&ss->hierarchy_mutex);
 			cgrp->subsys[i] = dummytop->subsys[i];
 			cgrp->subsys[i]->cgroup = cgrp;
 			list_add(&ss->sibling, &root->subsys_list);
 			rcu_assign_pointer(ss->root, root);
 			if (ss->bind)
 				ss->bind(ss, cgrp);
-
+			mutex_unlock(&ss->hierarchy_mutex);
 		} else if (bit & removed_bits) {
 			/* We're removing this subsystem */
 			BUG_ON(cgrp->subsys[i] != dummytop->subsys[i]);
 			BUG_ON(cgrp->subsys[i]->cgroup != cgrp);
+			mutex_lock(&ss->hierarchy_mutex);
 			if (ss->bind)
 				ss->bind(ss, dummytop);
 			dummytop->subsys[i]->cgroup = dummytop;
 			cgrp->subsys[i] = NULL;
 			rcu_assign_pointer(subsys[i]->root, &rootnode);
 			list_del(&ss->sibling);
+			mutex_unlock(&ss->hierarchy_mutex);
 		} else if (bit & final_bits) {
 			/* Subsystem state should already exist */
 			BUG_ON(!cgrp->subsys[i]);

@@ -2291,6 +2294,29 @@ static void init_cgroup_css(struct cgrou
 	cgrp->subsys[ss->subsys_id] = css;
 }
 
+static void cgroup_lock_hierarchy(struct cgroupfs_root *root)
+{
+	/* We need to take each hierarchy_mutex in a consistent order */
+	int i;
+
+	for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
+		struct cgroup_subsys *ss = subsys[i];
+		if (ss->root == root)
+			mutex_lock_nested(&ss->hierarchy_mutex, i);
+	}
+}
+
+static void cgroup_unlock_hierarchy(struct cgroupfs_root *root)
+{
+	int i;
+
+	for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
+		struct cgroup_subsys *ss = subsys[i];
+		if (ss->root == root)
+			mutex_unlock(&ss->hierarchy_mutex);
+	}
+}
+
 /*
  * cgroup_create - create a cgroup
  * @parent: cgroup that will be parent of the new cgroup

@@ -2342,8 +2368,10 @@ static long cgroup_create(struct cgroup 
 		init_cgroup_css(css, ss, cgrp);
 	}
 
+	cgroup_lock_hierarchy(root);
 	list_add(&cgrp->sibling, &cgrp->parent->children);
 	root->number_of_cgroups++;
+	cgroup_unlock_hierarchy(root);
 
 	err = cgroup_create_dir(cgrp, dentry, mode);
 	if (err < 0)

@@ -2460,8 +2488,12 @@ static int cgroup_rmdir(struct inode *un
 	if (!list_empty(&cgrp->release_list))
 		list_del(&cgrp->release_list);
 	spin_unlock(&release_list_lock);
+
+	cgroup_lock_hierarchy(root);
 	/* delete my sibling from parent->children */
 	list_del(&cgrp->sibling);
+	cgroup_unlock_hierarchy(root);
+
 	spin_lock(&cgrp->dentry->d_lock);
 	d = dget(cgrp->dentry);
 	cgrp->dentry = NULL;

@@ -2504,6 +2536,7 @@ static void __init cgroup_init_subsys(st
 	 * need to invoke fork callbacks here. */
 	BUG_ON(!list_empty(&init_task.tasks));
 
+	mutex_init(&ss->hierarchy_mutex);
 	ss->active = 1;
 }
kernel/cpuset.c +2 -2
Index: lockfix-2.6.26-rc5-mm3/kernel/cpuset.c
===================================================================
@@ -1929,7 +1929,7 @@ static void scan_for_empty_cpusets(const
 
 static void common_cpu_mem_hotplug_unplug(void)
 {
-	cgroup_lock();
+	mutex_lock(&cpuset_subsys.hierarchy_mutex);
 
 	top_cpuset.cpus_allowed = cpu_online_map;
 	top_cpuset.mems_allowed = node_states[N_HIGH_MEMORY];

@@ -1941,7 +1941,7 @@ static void common_cpu_mem_hotplug_unplu
 	 */
 	rebuild_sched_domains();
 
-	cgroup_unlock();
+	mutex_unlock(&cpuset_subsys.hierarchy_mutex);
 }
 
 /*
author
#1 lore raw
Quoted 5 lines · Paul Menage

This patch adds a hierarchy_mutex to the cgroup_subsys object that protects changes to the hierarchy observed by that subsystem. It is taken by the cgroup subsystem for the following operations:

Quoted 8 lines

- linking a cgroup into that subsystem's cgroup tree
- unlinking a cgroup from that subsystem's cgroup tree
- moving the subsystem to/from a hierarchy (including across the bind() callback)

Thus if the subsystem holds its own hierarchy_mutex, it can safely traverse its ts own hierarchy.

It struck me that there's a small race in this code now that we're not doing cgroup_lock() in the hotplug path.

- we start to attach a task T to cpuset C, with a single CPU "X" in its "cpus" list
- cpuset_can_attach() returns successfully since the cpuset has a cpu
- CPU X gets hot-unplugged; any tasks in C are moved to their parent cpuset and C loses its cpu.
- we update T->cgroups to point to C, which is broken since C has no cpus.

So we'll need some additional locking work on top of this - but I think this patch is still a step in the right direction.

Paul

#2 lore raw

Hi Paul,

Sorry for the delay.

Quoted 24 lines · Paul Menage
Quoted 4 lines · Paul Menage

This patch adds a hierarchy_mutex to the cgroup_subsys object that protects changes to the hierarchy observed by that subsystem. It is taken by the cgroup subsystem for the following operations:

Quoted 8 lines

- linking a cgroup into that subsystem's cgroup tree
- unlinking a cgroup from that subsystem's cgroup tree
- moving the subsystem to/from a hierarchy (including across the bind() callback)

Thus if the subsystem holds its own hierarchy_mutex, it can safely traverse its ts own hierarchy.

It struck me that there's a small race in this code now that we're not doing cgroup_lock() in the hotplug path.

- we start to attach a task T to cpuset C, with a single CPU "X" in its "cpus" list
- cpuset_can_attach() returns successfully since the cpuset has a cpu
- CPU X gets hot-unplugged; any tasks in C are moved to their parent cpuset and C loses its cpu.
- we update T->cgroups to point to C, which is broken since C has no cpus.

Quoted 2 lines

So we'll need some additional locking work on top of this - but I think this patch is still a step in the right direction.

I was about to say "yeah, looks good" and then tried a couple of different hot-plug scenarious.
We still have circular locking even with your patch

[ INFO: possible circular locking dependency detected ] 2.6.26-rc8 #4
-------------------------------------------------------
bash/2779 is trying to acquire lock:
(&cpu_hotplug.lock){--..}, at: [<ffffffff8025e024>] get_online_cpus+0x24/0x40

but task is already holding lock:
(sched_domains_mutex){--..}, at: [<ffffffff8022f5e9>]
partition_sched_domains+0x29/0x2b0

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #2 (sched_domains_mutex){--..}:

       [<ffffffff8025988f>] __lock_acquire+0x9cf/0xe50
       [<ffffffff80259d6b>] lock_acquire+0x5b/0x80
       [<ffffffff804d12c4>] mutex_lock_nested+0x94/0x250
       [<ffffffff8022f5e9>] partition_sched_domains+0x29/0x2b0
       [<ffffffff80268f9d>] rebuild_sched_domains+0x9d/0x3f0
       [<ffffffff80269f05>] cpuset_handle_cpuhp+0x205/0x220
       [<ffffffff804d688f>] notifier_call_chain+0x3f/0x80
       [<ffffffff80250679>] __raw_notifier_call_chain+0x9/0x10
       [<ffffffff804c1748>] _cpu_down+0xa8/0x290
       [<ffffffff804c196b>] cpu_down+0x3b/0x60
       [<ffffffff804c2c68>] store_online+0x48/0xa0
       [<ffffffff803a46c4>] sysdev_store+0x24/0x30
       [<ffffffff802eebba>] sysfs_write_file+0xca/0x140
       [<ffffffff8029cb3b>] vfs_write+0xcb/0x170
       [<ffffffff8029ccd0>] sys_write+0x50/0x90
       [<ffffffff8020b92b>] system_call_after_swapgs+0x7b/0x80
       [<ffffffffffffffff>] 0xffffffffffffffff

-> #1 (&ss->hierarchy_mutex){--..}:

       [<ffffffff8025988f>] __lock_acquire+0x9cf/0xe50
       [<ffffffff80259d6b>] lock_acquire+0x5b/0x80
       [<ffffffff804d12c4>] mutex_lock_nested+0x94/0x250
       [<ffffffff80269d39>] cpuset_handle_cpuhp+0x39/0x220
       [<ffffffff804d688f>] notifier_call_chain+0x3f/0x80
       [<ffffffff80250679>] __raw_notifier_call_chain+0x9/0x10
       [<ffffffff804c1748>] _cpu_down+0xa8/0x290
       [<ffffffff804c196b>] cpu_down+0x3b/0x60
       [<ffffffff804c2c68>] store_online+0x48/0xa0
       [<ffffffff803a46c4>] sysdev_store+0x24/0x30
       [<ffffffff802eebba>] sysfs_write_file+0xca/0x140
       [<ffffffff8029cb3b>] vfs_write+0xcb/0x170
       [<ffffffff8029ccd0>] sys_write+0x50/0x90
       [<ffffffff8020b92b>] system_call_after_swapgs+0x7b/0x80
       [<ffffffffffffffff>] 0xffffffffffffffff

-> #0 (&cpu_hotplug.lock){--..}:

       [<ffffffff80259913>] __lock_acquire+0xa53/0xe50
       [<ffffffff80259d6b>] lock_acquire+0x5b/0x80
       [<ffffffff804d12c4>] mutex_lock_nested+0x94/0x250
       [<ffffffff8025e024>] get_online_cpus+0x24/0x40
       [<ffffffff8022fee1>] sched_getaffinity+0x11/0x80
       [<ffffffff8026e6d9>] __synchronize_sched+0x19/0x90
       [<ffffffff8022ed46>] detach_destroy_domains+0x46/0x50
       [<ffffffff8022f6b9>] partition_sched_domains+0xf9/0x2b0
       [<ffffffff80268f9d>] rebuild_sched_domains+0x9d/0x3f0
       [<ffffffff8026a858>] cpuset_common_file_write+0x2b8/0x5c0
       [<ffffffff8026657c>] cgroup_file_write+0x7c/0x1a0
       [<ffffffff8029cb3b>] vfs_write+0xcb/0x170
       [<ffffffff8029ccd0>] sys_write+0x50/0x90
       [<ffffffff8020b92b>] system_call_after_swapgs+0x7b/0x80
       [<ffffffffffffffff>] 0xffffffffffffffff

other info that might help us debug this:

2 locks held by bash/2779:
#0: (cgroup_mutex){--..}, at: [<ffffffff802653e2>] cgroup_lock+0x12/0x20 #1: (sched_domains_mutex){--..}, at: [<ffffffff8022f5e9>]

partition_sched_domains+0x29/0x2b0

stack backtrace:

Pid: 2779, comm: bash Not tainted 2.6.26-rc8 #4

Call Trace:
 [<ffffffff80258c0c>] print_circular_bug_tail+0x8c/0x90
 [<ffffffff802589c4>] ? print_circular_bug_entry+0x54/0x60
 [<ffffffff80259913>] __lock_acquire+0xa53/0xe50
 [<ffffffff8025e024>] ? get_online_cpus+0x24/0x40
 [<ffffffff80259d6b>] lock_acquire+0x5b/0x80
 [<ffffffff8025e024>] ? get_online_cpus+0x24/0x40
 [<ffffffff804d12c4>] mutex_lock_nested+0x94/0x250
 [<ffffffff8025867d>] ? mark_held_locks+0x4d/0x90
 [<ffffffff8025e024>] get_online_cpus+0x24/0x40
 [<ffffffff8022fee1>] sched_getaffinity+0x11/0x80
 [<ffffffff8026e6d9>] __synchronize_sched+0x19/0x90
 [<ffffffff8022ed46>] detach_destroy_domains+0x46/0x50
 [<ffffffff8022f6b9>] partition_sched_domains+0xf9/0x2b0
 [<ffffffff80258801>] ? trace_hardirqs_on+0xc1/0xe0
 [<ffffffff80268f9d>] rebuild_sched_domains+0x9d/0x3f0
 [<ffffffff8026a858>] cpuset_common_file_write+0x2b8/0x5c0
 [<ffffffff80268c00>] ? cpuset_test_cpumask+0x0/0x20
 [<ffffffff80269f20>] ? cpuset_change_cpumask+0x0/0x20
 [<ffffffff80265260>] ? started_after+0x0/0x50
 [<ffffffff8026657c>] cgroup_file_write+0x7c/0x1a0
 [<ffffffff8029cb3b>] vfs_write+0xcb/0x170
 [<ffffffff8029ccd0>] sys_write+0x50/0x90
 [<ffffffff8020b92b>] system_call_after_swapgs+0x7b/0x80

CPU3 attaching NULL sched-domain.

author
#3 lore raw
Quoted 4 lines · Max Krasnyansky

I was about to say "yeah, looks good" and then tried a couple of different hot-plug scenarious.
We still have circular locking even with your patch

What sequence of actions do you do? I've not been able to reproduce a lockdep failure.

Paul

Quoted 107 lines

[ INFO: possible circular locking dependency detected ] 2.6.26-rc8 #4
-------------------------------------------------------
bash/2779 is trying to acquire lock:
(&cpu_hotplug.lock){--..}, at: [<ffffffff8025e024>] get_online_cpus+0x24/0x40

but task is already holding lock:
(sched_domains_mutex){--..}, at: [<ffffffff8022f5e9>]
partition_sched_domains+0x29/0x2b0

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #2 (sched_domains_mutex){--..}:

      [<ffffffff8025988f>] __lock_acquire+0x9cf/0xe50
      [<ffffffff80259d6b>] lock_acquire+0x5b/0x80
      [<ffffffff804d12c4>] mutex_lock_nested+0x94/0x250
      [<ffffffff8022f5e9>] partition_sched_domains+0x29/0x2b0
      [<ffffffff80268f9d>] rebuild_sched_domains+0x9d/0x3f0
      [<ffffffff80269f05>] cpuset_handle_cpuhp+0x205/0x220
      [<ffffffff804d688f>] notifier_call_chain+0x3f/0x80
      [<ffffffff80250679>] __raw_notifier_call_chain+0x9/0x10
      [<ffffffff804c1748>] _cpu_down+0xa8/0x290
      [<ffffffff804c196b>] cpu_down+0x3b/0x60
      [<ffffffff804c2c68>] store_online+0x48/0xa0
      [<ffffffff803a46c4>] sysdev_store+0x24/0x30
      [<ffffffff802eebba>] sysfs_write_file+0xca/0x140
      [<ffffffff8029cb3b>] vfs_write+0xcb/0x170
      [<ffffffff8029ccd0>] sys_write+0x50/0x90
      [<ffffffff8020b92b>] system_call_after_swapgs+0x7b/0x80
      [<ffffffffffffffff>] 0xffffffffffffffff

-> #1 (&ss->hierarchy_mutex){--..}:

      [<ffffffff8025988f>] __lock_acquire+0x9cf/0xe50
      [<ffffffff80259d6b>] lock_acquire+0x5b/0x80
      [<ffffffff804d12c4>] mutex_lock_nested+0x94/0x250
      [<ffffffff80269d39>] cpuset_handle_cpuhp+0x39/0x220
      [<ffffffff804d688f>] notifier_call_chain+0x3f/0x80
      [<ffffffff80250679>] __raw_notifier_call_chain+0x9/0x10
      [<ffffffff804c1748>] _cpu_down+0xa8/0x290
      [<ffffffff804c196b>] cpu_down+0x3b/0x60
      [<ffffffff804c2c68>] store_online+0x48/0xa0
      [<ffffffff803a46c4>] sysdev_store+0x24/0x30
      [<ffffffff802eebba>] sysfs_write_file+0xca/0x140
      [<ffffffff8029cb3b>] vfs_write+0xcb/0x170
      [<ffffffff8029ccd0>] sys_write+0x50/0x90
      [<ffffffff8020b92b>] system_call_after_swapgs+0x7b/0x80
      [<ffffffffffffffff>] 0xffffffffffffffff

-> #0 (&cpu_hotplug.lock){--..}:

      [<ffffffff80259913>] __lock_acquire+0xa53/0xe50
      [<ffffffff80259d6b>] lock_acquire+0x5b/0x80
      [<ffffffff804d12c4>] mutex_lock_nested+0x94/0x250
      [<ffffffff8025e024>] get_online_cpus+0x24/0x40
      [<ffffffff8022fee1>] sched_getaffinity+0x11/0x80
      [<ffffffff8026e6d9>] __synchronize_sched+0x19/0x90
      [<ffffffff8022ed46>] detach_destroy_domains+0x46/0x50
      [<ffffffff8022f6b9>] partition_sched_domains+0xf9/0x2b0
      [<ffffffff80268f9d>] rebuild_sched_domains+0x9d/0x3f0
      [<ffffffff8026a858>] cpuset_common_file_write+0x2b8/0x5c0
      [<ffffffff8026657c>] cgroup_file_write+0x7c/0x1a0
      [<ffffffff8029cb3b>] vfs_write+0xcb/0x170
      [<ffffffff8029ccd0>] sys_write+0x50/0x90
      [<ffffffff8020b92b>] system_call_after_swapgs+0x7b/0x80
      [<ffffffffffffffff>] 0xffffffffffffffff

other info that might help us debug this:

2 locks held by bash/2779:
#0: (cgroup_mutex){--..}, at: [<ffffffff802653e2>] cgroup_lock+0x12/0x20 #1: (sched_domains_mutex){--..}, at: [<ffffffff8022f5e9>]

partition_sched_domains+0x29/0x2b0

stack backtrace:

Pid: 2779, comm: bash Not tainted 2.6.26-rc8 #4

Call Trace:
 [<ffffffff80258c0c>] print_circular_bug_tail+0x8c/0x90
 [<ffffffff802589c4>] ? print_circular_bug_entry+0x54/0x60
 [<ffffffff80259913>] __lock_acquire+0xa53/0xe50
 [<ffffffff8025e024>] ? get_online_cpus+0x24/0x40
 [<ffffffff80259d6b>] lock_acquire+0x5b/0x80
 [<ffffffff8025e024>] ? get_online_cpus+0x24/0x40
 [<ffffffff804d12c4>] mutex_lock_nested+0x94/0x250
 [<ffffffff8025867d>] ? mark_held_locks+0x4d/0x90
 [<ffffffff8025e024>] get_online_cpus+0x24/0x40
 [<ffffffff8022fee1>] sched_getaffinity+0x11/0x80
 [<ffffffff8026e6d9>] __synchronize_sched+0x19/0x90
 [<ffffffff8022ed46>] detach_destroy_domains+0x46/0x50
 [<ffffffff8022f6b9>] partition_sched_domains+0xf9/0x2b0
 [<ffffffff80258801>] ? trace_hardirqs_on+0xc1/0xe0
 [<ffffffff80268f9d>] rebuild_sched_domains+0x9d/0x3f0
 [<ffffffff8026a858>] cpuset_common_file_write+0x2b8/0x5c0
 [<ffffffff80268c00>] ? cpuset_test_cpumask+0x0/0x20
 [<ffffffff80269f20>] ? cpuset_change_cpumask+0x0/0x20
 [<ffffffff80265260>] ? started_after+0x0/0x50
 [<ffffffff8026657c>] cgroup_file_write+0x7c/0x1a0
 [<ffffffff8029cb3b>] vfs_write+0xcb/0x170
 [<ffffffff8029ccd0>] sys_write+0x50/0x90
 [<ffffffff8020b92b>] system_call_after_swapgs+0x7b/0x80

CPU3 attaching NULL sched-domain.

#4 lore raw
Quoted 8 lines · Paul Menage
Quoted 4 lines · Max Krasnyansky

I was about to say "yeah, looks good" and then tried a couple of different hot-plug scenarious.
We still have circular locking even with your patch

What sequence of actions do you do? I've not been able to reproduce a lockdep failure.

mkdir /dev/cpuset
mount -t cgroup -o cpuset cpuset /dev/cpuset
mkdir /dev/cpuset/0
mkdir /dev/cpuset/1
echo 0-2 > /dev/cpuset/0/cpuset.cpus
echo 3 > /dev/cpuset/1/cpuset.cpus
echo 0 > /dev/cpuset/cpuset.sched_load_balance
echo 0 > /sys/devices/system/cpu/cpu3/online

I just tried it again and got exact same lockdep output that I sent before.

Max

#5 lore raw
Quoted 19 lines · Max Krasnyansky
Quoted 7 lines · Paul Menage
Quoted 4 lines · Max Krasnyansky

I was about to say "yeah, looks good" and then tried a couple of different hot-plug scenarious.
We still have circular locking even with your patch

What sequence of actions do you do? I've not been able to reproduce a lockdep failure.

mkdir /dev/cpuset
mount -t cgroup -o cpuset cpuset /dev/cpuset
mkdir /dev/cpuset/0
mkdir /dev/cpuset/1
echo 0-2 > /dev/cpuset/0/cpuset.cpus
echo 3 > /dev/cpuset/1/cpuset.cpus
echo 0 > /dev/cpuset/cpuset.sched_load_balance
echo 0 > /sys/devices/system/cpu/cpu3/online

Quoted 1 line

I just tried it again and got exact same lockdep output that I sent before.

Paul, any luck with that ?

Max

author
#6 lore raw
In reply to Max Krasnyansky
Quoted 10 lines · Max Krasnyansky

mkdir /dev/cpuset
mount -t cgroup -o cpuset cpuset /dev/cpuset
mkdir /dev/cpuset/0
mkdir /dev/cpuset/1
echo 0-2 > /dev/cpuset/0/cpuset.cpus
echo 3 > /dev/cpuset/1/cpuset.cpus
echo 0 > /dev/cpuset/cpuset.sched_load_balance
echo 0 > /sys/devices/system/cpu/cpu3/online

OK, I still can't reproduce this, on a 2-cpu system using one cpu for each cpuset.

But the basic problem seemns to be that we have cpu_hotplug.lock taken at the outer level (when offlining a CPU) and at the inner level (via get_online_cpus() called from the guts of partition_sched_domains(), if we didn't already take it at the outer level.

While looking at the code trying to figure out a nice way around this, it struck me that we have the call path

cpuset_track_online_nodes() ->
common_cpu_mem_hotplug_unplug() ->
scan_for_empty_cpusets() ->
access cpu_online_map with no calls to get_online_cpus()

Is that allowed? Maybe we need separate versions of scan_for_empty_cpusets() that look at memory and cpus?

I think that we're going to want eventually a solution such pushing the locking of cpuset_subsys.hierarchy_mutex down into the first part of partition_sched_domains, that actually walks the cpuset tree

Paul

#7 lore raw
Quoted 11 lines · Paul Menage
Quoted 9 lines · Max Krasnyansky

mkdir /dev/cpuset
mount -t cgroup -o cpuset cpuset /dev/cpuset
mkdir /dev/cpuset/0
mkdir /dev/cpuset/1
echo 0-2 > /dev/cpuset/0/cpuset.cpus
echo 3 > /dev/cpuset/1/cpuset.cpus
echo 0 > /dev/cpuset/cpuset.sched_load_balance
echo 0 > /sys/devices/system/cpu/cpu3/online

Quoted 7 lines

OK, I still can't reproduce this, on a 2-cpu system using one cpu for each cpuset.

But the basic problem seemns to be that we have cpu_hotplug.lock taken at the outer level (when offlining a CPU) and at the inner level (via get_online_cpus() called from the guts of partition_sched_domains(), if we didn't already take it at the outer level.

Yes. I'm looking at this stuff right now.

Quoted 10 lines

While looking at the code trying to figure out a nice way around this, it struck me that we have the call path

cpuset_track_online_nodes() ->
common_cpu_mem_hotplug_unplug() ->
scan_for_empty_cpusets() ->
access cpu_online_map with no calls to get_online_cpus()

Is that allowed? Maybe we need separate versions of scan_for_empty_cpusets() that look at memory and cpus?

Yes. This also came up in the other discussions. ie That we need to have this split up somehow. Looks like my fix for domain handling exposed all kinds of problems that we punted on before :).

Quoted 3 lines

I think that we're going to want eventually a solution such pushing the locking of cpuset_subsys.hierarchy_mutex down into the first part of partition_sched_domains, that actually walks the cpuset tree

Right now I'm trying to get away with not using new locks. Since we need some solution for 2.6.26 and new cgroup locking that you added is probably a .27 material.

Max