[PATCH 0/6] cpuset: convert to new cpumask API

[PATCH 0/6] cpuset: convert to new cpumask API

[Li Zefan]

This patchset converts cpuset to use new cpumask API, and thus
remove on stack cpumask_t to reduce stack usage.

Before:
 # cat kernel/cpuset.c include/linux/cpuset.h | grep -c cpumask_t
 21
After:
 # cat kernel/cpuset.c include/linux/cpuset.h | grep -c cpumask_t
 0

The patchset is based on mmotm 2008-12-30-16-05.

[PATCH 1/6] cpuset: remove on stack cpumask_t in cpuset_sprintf_cpulist()
[PATCH 2/6] cpuset: remove on stack cpumask_t in cpuset_can_attach()
[PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t
[PATCH 4/6] cpuset: don't allocate trial cpuset on stack
[PATCH 5/6] cpuset: convert cpuset->cpus_allowed to cpumask_var_t
[PATCH 6/6] cpuset: remove remaining pointers to cpumask_t
---
 include/linux/cpuset.h   |   10 +
 kernel/cpuset.c          |  279 ++++++++++++++++++++++++++++-------------------
 3 files changed, 174 insertions(+), 115 deletions(-)

[PATCH 1/6] cpuset: remove on stack cpumask_t in cpuset_sprintf_cpulist()

[Li Zefan]

Impact: reduce stack usage

It's safe to call cpulist_scnprintf inside callback_mutex, and thus we
can just remove the cpumask_t and no need to allocate a cpumask_var_t.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/cpuset.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 6012e32..41c2343 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1486,13 +1486,13 @@ static int cpuset_write_resmask(struct cgroup *cgrp, struct cftype *cft,
 
 static int cpuset_sprintf_cpulist(char *page, struct cpuset *cs)
 {
-	cpumask_t mask;
+	int ret;
 
 	mutex_lock(&callback_mutex);
-	mask = cs->cpus_allowed;
+	ret = cpulist_scnprintf(page, PAGE_SIZE, &cs->cpus_allowed);
 	mutex_unlock(&callback_mutex);
 
-	return cpulist_scnprintf(page, PAGE_SIZE, &mask);
+	return ret;
 }
 
 static int cpuset_sprintf_memlist(char *page, struct cpuset *cs)
-- 
1.5.4.rc3

[PATCH 2/6] cpuset: remove on stack cpumask_t in cpuset_can_attach()

[Li Zefan]

Impact: redue stack usage

Just use cs->cpus_allowed, and no need to allocate a cpumask_var_t.

Signed-off-by: Li Zefan <lizf@cn.fujistu.com>
---
 kernel/cpuset.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 41c2343..afa29cf 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1311,20 +1311,19 @@ static int cpuset_can_attach(struct cgroup_subsys *ss,
 			     struct cgroup *cont, struct task_struct *tsk)
 {
 	struct cpuset *cs = cgroup_cs(cont);
+	int ret = 0;
 
 	if (cpus_empty(cs->cpus_allowed) || nodes_empty(cs->mems_allowed))
 		return -ENOSPC;
-	if (tsk->flags & PF_THREAD_BOUND) {
-		cpumask_t mask;
 
+	if (tsk->flags & PF_THREAD_BOUND) {
 		mutex_lock(&callback_mutex);
-		mask = cs->cpus_allowed;
+		if (!cpus_equal(tsk->cpus_allowed, cs->cpus_allowed))
+			ret = -EINVAL;
 		mutex_unlock(&callback_mutex);
-		if (!cpus_equal(tsk->cpus_allowed, mask))
-			return -EINVAL;
 	}
 
-	return security_task_setscheduler(tsk, 0, NULL);
+	return ret < 0 ? ret : security_task_setscheduler(tsk, 0, NULL);
 }
 
 static void cpuset_attach(struct cgroup_subsys *ss,
-- 
1.5.4.rc3

[PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

[Li Zefan]

Impact: reduce stack usage

Allocate a cpumask_var_t in cpuset_can_attach() and then use it in
cpuset_attach(), so we won't fail cpuset_attach().

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/cpuset.c |   31 +++++++++++++++++++++++++------
 1 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index afa29cf..b4c36d5 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1306,6 +1306,14 @@ static int fmeter_getrate(struct fmeter *fmp)
 	return val;
 }
 
+/*
+ * Since cpuset_attach() can't fail, we allocate a cpumask_var_t in
+ * cpuset_can_attach() and then use it in cpuset_attach().
+ *
+ * Protected by cgroup_lock.
+ */
+static cpumask_var_t cpus_attach;
+
 /* Called by cgroups to determine if a cpuset is usable; cgroup_mutex held */
 static int cpuset_can_attach(struct cgroup_subsys *ss,
 			     struct cgroup *cont, struct task_struct *tsk)
@@ -1321,16 +1329,25 @@ static int cpuset_can_attach(struct cgroup_subsys *ss,
 		if (!cpus_equal(tsk->cpus_allowed, cs->cpus_allowed))
 			ret = -EINVAL;
 		mutex_unlock(&callback_mutex);
+
+		if (ret)
+			return ret;
 	}
 
-	return ret < 0 ? ret : security_task_setscheduler(tsk, 0, NULL);
+	ret = security_task_setscheduler(tsk, 0, NULL);
+	if (ret)
+		return ret;
+
+	if (!alloc_cpumask_var(&cpus_attach, GFP_KERNEL))
+		return -ENOMEM;
+
+	return 0;
 }
 
 static void cpuset_attach(struct cgroup_subsys *ss,
 			  struct cgroup *cont, struct cgroup *oldcont,
 			  struct task_struct *tsk)
 {
-	cpumask_t cpus;
 	nodemask_t from, to;
 	struct mm_struct *mm;
 	struct cpuset *cs = cgroup_cs(cont);
@@ -1338,15 +1355,15 @@ static void cpuset_attach(struct cgroup_subsys *ss,
 	int err;
 
 	if (cs == &top_cpuset) {
-		cpus = cpu_possible_map;
+		cpumask_copy(cpus_attach, cpu_possible_mask);
 	} else {
 		mutex_lock(&callback_mutex);
-		guarantee_online_cpus(cs, &cpus);
+		guarantee_online_cpus(cs, cpus_attach);
 		mutex_unlock(&callback_mutex);
 	}
-	err = set_cpus_allowed_ptr(tsk, &cpus);
+	err = set_cpus_allowed_ptr(tsk, cpus_attach);
 	if (err)
-		return;
+		goto out;
 
 	from = oldcs->mems_allowed;
 	to = cs->mems_allowed;
@@ -1358,6 +1375,8 @@ static void cpuset_attach(struct cgroup_subsys *ss,
 		mmput(mm);
 	}
 
+out:
+	free_cpumask_var(cpus_attach);
 }
 
 /* The various types of files and directories in a cpuset file system */
-- 
1.5.4.rc3

[PATCH 4/6] cpuset: don't allocate trial cpuset on stack

[Li Zefan]

Impact: cleanups, reduce stack usage

This patch prepares for the next patch. When we convert cpuset.cpus_allowed
to cpumask_var_t, (trialcs = *cs) no longer works.

Another result of this patch is reducing stack usage of trialcs. sizeof(*cs)
can be as large as 148 bytes on x86_64, so it's really not good to have it
on stack.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/cpuset.c |   93 +++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 60 insertions(+), 33 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index b4c36d5..480932a 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -415,6 +415,24 @@ static int is_cpuset_subset(const struct cpuset *p, const struct cpuset *q)
 		is_mem_exclusive(p) <= is_mem_exclusive(q);
 }
 
+/**
+ * alloc_trial_cpuset - allocate a trial cpuset
+ * @cs: the cpuset that the trial cpuset duplicates
+ */
+static struct cpuset *alloc_trial_cpuset(const struct cpuset *cs)
+{
+	return kmemdup(cs, sizeof(*cs), GFP_KERNEL);
+}
+
+/**
+ * free_trial_cpuset - free the trial cpuset
+ * @trial: the trial cpuset to be freed
+ */
+static void free_trial_cpuset(struct cpuset *trial)
+{
+	kfree(trial);
+}
+
 /*
  * validate_change() - Used to validate that any proposed cpuset change
  *		       follows the structural rules for cpusets.
@@ -880,10 +898,10 @@ static void update_tasks_cpumask(struct cpuset *cs, struct ptr_heap *heap)
  * @cs: the cpuset to consider
  * @buf: buffer of cpu numbers written to this cpuset
  */
-static int update_cpumask(struct cpuset *cs, const char *buf)
+static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
+			  const char *buf)
 {
 	struct ptr_heap heap;
-	struct cpuset trialcs;
 	int retval;
 	int is_load_balanced;
 
@@ -891,8 +909,6 @@ static int update_cpumask(struct cpuset *cs, const char *buf)
 	if (cs == &top_cpuset)
 		return -EACCES;
 
-	trialcs = *cs;
-
 	/*
 	 * An empty cpus_allowed is ok only if the cpuset has no tasks.
 	 * Since cpulist_parse() fails on an empty mask, we special case
@@ -900,31 +916,31 @@ static int update_cpumask(struct cpuset *cs, const char *buf)
 	 * with tasks have cpus.
 	 */
 	if (!*buf) {
-		cpus_clear(trialcs.cpus_allowed);
+		cpus_clear(trialcs->cpus_allowed);
 	} else {
-		retval = cpulist_parse(buf, &trialcs.cpus_allowed);
+		retval = cpulist_parse(buf, &trialcs->cpus_allowed);
 		if (retval < 0)
 			return retval;
 
-		if (!cpus_subset(trialcs.cpus_allowed, cpu_online_map))
+		if (!cpus_subset(trialcs->cpus_allowed, cpu_online_map))
 			return -EINVAL;
 	}
-	retval = validate_change(cs, &trialcs);
+	retval = validate_change(cs, trialcs);
 	if (retval < 0)
 		return retval;
 
 	/* Nothing to do if the cpus didn't change */
-	if (cpus_equal(cs->cpus_allowed, trialcs.cpus_allowed))
+	if (cpus_equal(cs->cpus_allowed, trialcs->cpus_allowed))
 		return 0;
 
 	retval = heap_init(&heap, PAGE_SIZE, GFP_KERNEL, NULL);
 	if (retval)
 		return retval;
 
-	is_load_balanced = is_sched_load_balance(&trialcs);
+	is_load_balanced = is_sched_load_balance(trialcs);
 
 	mutex_lock(&callback_mutex);
-	cs->cpus_allowed = trialcs.cpus_allowed;
+	cs->cpus_allowed = trialcs->cpus_allowed;
 	mutex_unlock(&callback_mutex);
 
 	/*
@@ -1099,9 +1115,9 @@ done:
  * lock each such tasks mm->mmap_sem, scan its vma's and rebind
  * their mempolicies to the cpusets new mems_allowed.
  */
-static int update_nodemask(struct cpuset *cs, const char *buf)
+static int update_nodemask(struct cpuset *cs, struct cpuset *trialcs,
+			   const char *buf)
 {
-	struct cpuset trialcs;
 	nodemask_t oldmem;
 	int retval;
 
@@ -1112,8 +1128,6 @@ static int update_nodemask(struct cpuset *cs, const char *buf)
 	if (cs == &top_cpuset)
 		return -EACCES;
 
-	trialcs = *cs;
-
 	/*
 	 * An empty mems_allowed is ok iff there are no tasks in the cpuset.
 	 * Since nodelist_parse() fails on an empty mask, we special case
@@ -1121,27 +1135,27 @@ static int update_nodemask(struct cpuset *cs, const char *buf)
 	 * with tasks have memory.
 	 */
 	if (!*buf) {
-		nodes_clear(trialcs.mems_allowed);
+		nodes_clear(trialcs->mems_allowed);
 	} else {
-		retval = nodelist_parse(buf, trialcs.mems_allowed);
+		retval = nodelist_parse(buf, trialcs->mems_allowed);
 		if (retval < 0)
 			goto done;
 
-		if (!nodes_subset(trialcs.mems_allowed,
+		if (!nodes_subset(trialcs->mems_allowed,
 				node_states[N_HIGH_MEMORY]))
 			return -EINVAL;
 	}
 	oldmem = cs->mems_allowed;
-	if (nodes_equal(oldmem, trialcs.mems_allowed)) {
+	if (nodes_equal(oldmem, trialcs->mems_allowed)) {
 		retval = 0;		/* Too easy - nothing to do */
 		goto done;
 	}
-	retval = validate_change(cs, &trialcs);
+	retval = validate_change(cs, trialcs);
 	if (retval < 0)
 		goto done;
 
 	mutex_lock(&callback_mutex);
-	cs->mems_allowed = trialcs.mems_allowed;
+	cs->mems_allowed = trialcs->mems_allowed;
 	cs->mems_generation = cpuset_mems_generation++;
 	mutex_unlock(&callback_mutex);
 
@@ -1181,31 +1195,36 @@ static int update_relax_domain_level(struct cpuset *cs, s64 val)
 static int update_flag(cpuset_flagbits_t bit, struct cpuset *cs,
 		       int turning_on)
 {
-	struct cpuset trialcs;
+	struct cpuset *trialcs;
 	int err;
 	int balance_flag_changed;
 
-	trialcs = *cs;
+	trialcs = alloc_trial_cpuset(cs);
+	if (!trialcs)
+		return -ENOMEM;
+
 	if (turning_on)
-		set_bit(bit, &trialcs.flags);
+		set_bit(bit, &trialcs->flags);
 	else
-		clear_bit(bit, &trialcs.flags);
+		clear_bit(bit, &trialcs->flags);
 
-	err = validate_change(cs, &trialcs);
+	err = validate_change(cs, trialcs);
 	if (err < 0)
-		return err;
+		goto out;
 
 	balance_flag_changed = (is_sched_load_balance(cs) !=
-		 			is_sched_load_balance(&trialcs));
+				is_sched_load_balance(trialcs));
 
 	mutex_lock(&callback_mutex);
-	cs->flags = trialcs.flags;
+	cs->flags = trialcs->flags;
 	mutex_unlock(&callback_mutex);
 
-	if (!cpus_empty(trialcs.cpus_allowed) && balance_flag_changed)
+	if (!cpus_empty(trialcs->cpus_allowed) && balance_flag_changed)
 		async_rebuild_sched_domains();
 
-	return 0;
+out:
+	free_trial_cpuset(trialcs);
+	return err;
 }
 
 /*
@@ -1471,21 +1490,29 @@ static int cpuset_write_resmask(struct cgroup *cgrp, struct cftype *cft,
 				const char *buf)
 {
 	int retval = 0;
+	struct cpuset *cs = cgroup_cs(cgrp);
+	struct cpuset *trialcs;
 
 	if (!cgroup_lock_live_group(cgrp))
 		return -ENODEV;
 
+	trialcs = alloc_trial_cpuset(cs);
+	if (!trialcs)
+		return -ENOMEM;
+
 	switch (cft->private) {
 	case FILE_CPULIST:
-		retval = update_cpumask(cgroup_cs(cgrp), buf);
+		retval = update_cpumask(cs, trialcs, buf);
 		break;
 	case FILE_MEMLIST:
-		retval = update_nodemask(cgroup_cs(cgrp), buf);
+		retval = update_nodemask(cs, trialcs, buf);
 		break;
 	default:
 		retval = -EINVAL;
 		break;
 	}
+
+	free_trial_cpuset(trialcs);
 	cgroup_unlock();
 	return retval;
 }
-- 
1.5.4.rc3

[PATCH 5/6] cpuset: convert cpuset->cpus_allowed to cpumask_var_t

[Li Zefan]

Impact: use new cpumask API

This patch mainly does the following things:
- change cs->cpus_allowed from cpumask_t to cpumask_var_t
- call alloc_bootmem_cpumask_var() for top_cpuset in cpuset_init_early()
- call alloc_cpumask_var() for other cpusets
- replace cpus_xxx() to cpumask_xxx()

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/cpuset.c |  100 +++++++++++++++++++++++++++++++++----------------------
 1 files changed, 60 insertions(+), 40 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 480932a..25df51f 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -84,7 +84,7 @@ struct cpuset {
 	struct cgroup_subsys_state css;
 
 	unsigned long flags;		/* "unsigned long" so bitops work */
-	cpumask_t cpus_allowed;		/* CPUs allowed to tasks in cpuset */
+	cpumask_var_t cpus_allowed;	/* CPUs allowed to tasks in cpuset */
 	nodemask_t mems_allowed;	/* Memory Nodes allowed to tasks */
 
 	struct cpuset *parent;		/* my parent */
@@ -195,8 +195,6 @@ static int cpuset_mems_generation;
 
 static struct cpuset top_cpuset = {
 	.flags = ((1 << CS_CPU_EXCLUSIVE) | (1 << CS_MEM_EXCLUSIVE)),
-	.cpus_allowed = CPU_MASK_ALL,
-	.mems_allowed = NODE_MASK_ALL,
 };
 
 /*
@@ -278,7 +276,7 @@ static struct file_system_type cpuset_fs_type = {
 };
 
 /*
- * Return in *pmask the portion of a cpusets's cpus_allowed that
+ * Return in pmask the portion of a cpusets's cpus_allowed that
  * are online.  If none are online, walk up the cpuset hierarchy
  * until we find one that does have some online cpus.  If we get
  * all the way to the top and still haven't found any online cpus,
@@ -293,13 +291,13 @@ static struct file_system_type cpuset_fs_type = {
 
 static void guarantee_online_cpus(const struct cpuset *cs, cpumask_t *pmask)
 {
-	while (cs && !cpus_intersects(cs->cpus_allowed, cpu_online_map))
+	while (cs && !cpumask_intersects(cs->cpus_allowed, cpu_online_mask))
 		cs = cs->parent;
 	if (cs)
-		cpus_and(*pmask, cs->cpus_allowed, cpu_online_map);
+		cpumask_and(pmask, cs->cpus_allowed, cpu_online_mask);
 	else
-		*pmask = cpu_online_map;
-	BUG_ON(!cpus_intersects(*pmask, cpu_online_map));
+		cpumask_copy(pmask, cpu_online_mask);
+	BUG_ON(!cpumask_intersects(pmask, cpu_online_mask));
 }
 
 /*
@@ -409,7 +407,7 @@ void cpuset_update_task_memory_state(void)
 
 static int is_cpuset_subset(const struct cpuset *p, const struct cpuset *q)
 {
-	return	cpus_subset(p->cpus_allowed, q->cpus_allowed) &&
+	return	cpumask_subset(p->cpus_allowed, q->cpus_allowed) &&
 		nodes_subset(p->mems_allowed, q->mems_allowed) &&
 		is_cpu_exclusive(p) <= is_cpu_exclusive(q) &&
 		is_mem_exclusive(p) <= is_mem_exclusive(q);
@@ -421,7 +419,19 @@ static int is_cpuset_subset(const struct cpuset *p, const struct cpuset *q)
  */
 static struct cpuset *alloc_trial_cpuset(const struct cpuset *cs)
 {
-	return kmemdup(cs, sizeof(*cs), GFP_KERNEL);
+	struct cpuset *trial;
+
+	trial = kmemdup(cs, sizeof(*cs), GFP_KERNEL);
+	if (!trial)
+		return NULL;
+
+	if (!alloc_cpumask_var(&trial->cpus_allowed, GFP_KERNEL)) {
+		kfree(trial);
+		return NULL;
+	}
+	cpumask_copy(trial->cpus_allowed, cs->cpus_allowed);
+
+	return trial;
 }
 
 /**
@@ -430,6 +440,7 @@ static struct cpuset *alloc_trial_cpuset(const struct cpuset *cs)
  */
 static void free_trial_cpuset(struct cpuset *trial)
 {
+	free_cpumask_var(trial->cpus_allowed);
 	kfree(trial);
 }
 
@@ -482,7 +493,7 @@ static int validate_change(const struct cpuset *cur, const struct cpuset *trial)
 		c = cgroup_cs(cont);
 		if ((is_cpu_exclusive(trial) || is_cpu_exclusive(c)) &&
 		    c != cur &&
-		    cpus_intersects(trial->cpus_allowed, c->cpus_allowed))
+		    cpumask_intersects(trial->cpus_allowed, c->cpus_allowed))
 			return -EINVAL;
 		if ((is_mem_exclusive(trial) || is_mem_exclusive(c)) &&
 		    c != cur &&
@@ -492,7 +503,7 @@ static int validate_change(const struct cpuset *cur, const struct cpuset *trial)
 
 	/* Cpusets with tasks can't have empty cpus_allowed or mems_allowed */
 	if (cgroup_task_count(cur->css.cgroup)) {
-		if (cpus_empty(trial->cpus_allowed) ||
+		if (cpumask_empty(trial->cpus_allowed) ||
 		    nodes_empty(trial->mems_allowed)) {
 			return -ENOSPC;
 		}
@@ -507,7 +518,7 @@ static int validate_change(const struct cpuset *cur, const struct cpuset *trial)
  */
 static int cpusets_overlap(struct cpuset *a, struct cpuset *b)
 {
-	return cpus_intersects(a->cpus_allowed, b->cpus_allowed);
+	return cpumask_intersects(a->cpus_allowed, b->cpus_allowed);
 }
 
 static void
@@ -532,7 +543,7 @@ update_domain_attr_tree(struct sched_domain_attr *dattr, struct cpuset *c)
 		cp = list_first_entry(&q, struct cpuset, stack_list);
 		list_del(q.next);
 
-		if (cpus_empty(cp->cpus_allowed))
+		if (cpumask_empty(cp->cpus_allowed))
 			continue;
 
 		if (is_sched_load_balance(cp))
@@ -627,7 +638,7 @@ static int generate_sched_domains(cpumask_t **domains,
 			*dattr = SD_ATTR_INIT;
 			update_domain_attr_tree(dattr, &top_cpuset);
 		}
-		*doms = top_cpuset.cpus_allowed;
+		cpumask_copy(doms, top_cpuset.cpus_allowed);
 
 		ndoms = 1;
 		goto done;
@@ -646,7 +657,7 @@ static int generate_sched_domains(cpumask_t **domains,
 		cp = list_first_entry(&q, struct cpuset, stack_list);
 		list_del(q.next);
 
-		if (cpus_empty(cp->cpus_allowed))
+		if (cpumask_empty(cp->cpus_allowed))
 			continue;
 
 		/*
@@ -739,7 +750,7 @@ restart:
 			struct cpuset *b = csa[j];
 
 			if (apn == b->pn) {
-				cpus_or(*dp, *dp, b->cpus_allowed);
+				cpumask_or(dp, dp, b->cpus_allowed);
 				if (dattr)
 					update_domain_attr_tree(dattr + nslot, b);
 
@@ -848,7 +859,7 @@ void rebuild_sched_domains(void)
 static int cpuset_test_cpumask(struct task_struct *tsk,
 			       struct cgroup_scanner *scan)
 {
-	return !cpus_equal(tsk->cpus_allowed,
+	return !cpumask_equal(&tsk->cpus_allowed,
 			(cgroup_cs(scan->cg))->cpus_allowed);
 }
 
@@ -866,7 +877,7 @@ static int cpuset_test_cpumask(struct task_struct *tsk,
 static void cpuset_change_cpumask(struct task_struct *tsk,
 				  struct cgroup_scanner *scan)
 {
-	set_cpus_allowed_ptr(tsk, &((cgroup_cs(scan->cg))->cpus_allowed));
+	set_cpus_allowed_ptr(tsk, ((cgroup_cs(scan->cg))->cpus_allowed));
 }
 
 /**
@@ -916,13 +927,13 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 	 * with tasks have cpus.
 	 */
 	if (!*buf) {
-		cpus_clear(trialcs->cpus_allowed);
+		cpumask_clear(trialcs->cpus_allowed);
 	} else {
-		retval = cpulist_parse(buf, &trialcs->cpus_allowed);
+		retval = cpulist_parse(buf, trialcs->cpus_allowed);
 		if (retval < 0)
 			return retval;
 
-		if (!cpus_subset(trialcs->cpus_allowed, cpu_online_map))
+		if (!cpumask_subset(trialcs->cpus_allowed, cpu_online_mask))
 			return -EINVAL;
 	}
 	retval = validate_change(cs, trialcs);
@@ -930,7 +941,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 		return retval;
 
 	/* Nothing to do if the cpus didn't change */
-	if (cpus_equal(cs->cpus_allowed, trialcs->cpus_allowed))
+	if (cpumask_equal(cs->cpus_allowed, trialcs->cpus_allowed))
 		return 0;
 
 	retval = heap_init(&heap, PAGE_SIZE, GFP_KERNEL, NULL);
@@ -940,7 +951,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 	is_load_balanced = is_sched_load_balance(trialcs);
 
 	mutex_lock(&callback_mutex);
-	cs->cpus_allowed = trialcs->cpus_allowed;
+	cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
 	mutex_unlock(&callback_mutex);
 
 	/*
@@ -1028,7 +1039,7 @@ static int update_tasks_nodemask(struct cpuset *cs, const nodemask_t *oldmem)
 	cpuset_being_rebound = cs;		/* causes mpol_dup() rebind */
 
 	fudge = 10;				/* spare mmarray[] slots */
-	fudge += cpus_weight(cs->cpus_allowed);	/* imagine one fork-bomb/cpu */
+	fudge += cpumask_weight(cs->cpus_allowed);/* imagine 1 fork-bomb/cpu */
 	retval = -ENOMEM;
 
 	/*
@@ -1176,7 +1187,8 @@ static int update_relax_domain_level(struct cpuset *cs, s64 val)
 
 	if (val != cs->relax_domain_level) {
 		cs->relax_domain_level = val;
-		if (!cpus_empty(cs->cpus_allowed) && is_sched_load_balance(cs))
+		if (!cpumask_empty(cs->cpus_allowed) &&
+		    is_sched_load_balance(cs))
 			async_rebuild_sched_domains();
 	}
 
@@ -1219,7 +1231,7 @@ static int update_flag(cpuset_flagbits_t bit, struct cpuset *cs,
 	cs->flags = trialcs->flags;
 	mutex_unlock(&callback_mutex);
 
-	if (!cpus_empty(trialcs->cpus_allowed) && balance_flag_changed)
+	if (!cpumask_empty(trialcs->cpus_allowed) && balance_flag_changed)
 		async_rebuild_sched_domains();
 
 out:
@@ -1340,12 +1352,12 @@ static int cpuset_can_attach(struct cgroup_subsys *ss,
 	struct cpuset *cs = cgroup_cs(cont);
 	int ret = 0;
 
-	if (cpus_empty(cs->cpus_allowed) || nodes_empty(cs->mems_allowed))
+	if (cpumask_empty(cs->cpus_allowed) || nodes_empty(cs->mems_allowed))
 		return -ENOSPC;
 
 	if (tsk->flags & PF_THREAD_BOUND) {
 		mutex_lock(&callback_mutex);
-		if (!cpus_equal(tsk->cpus_allowed, cs->cpus_allowed))
+		if (!cpumask_equal(&tsk->cpus_allowed, cs->cpus_allowed))
 			ret = -EINVAL;
 		mutex_unlock(&callback_mutex);
 
@@ -1534,7 +1546,7 @@ static int cpuset_sprintf_cpulist(char *page, struct cpuset *cs)
 	int ret;
 
 	mutex_lock(&callback_mutex);
-	ret = cpulist_scnprintf(page, PAGE_SIZE, &cs->cpus_allowed);
+	ret = cpulist_scnprintf(page, PAGE_SIZE, cs->cpus_allowed);
 	mutex_unlock(&callback_mutex);
 
 	return ret;
@@ -1773,7 +1785,7 @@ static void cpuset_post_clone(struct cgroup_subsys *ss,
 	parent_cs = cgroup_cs(parent);
 
 	cs->mems_allowed = parent_cs->mems_allowed;
-	cs->cpus_allowed = parent_cs->cpus_allowed;
+	cpumask_copy(cs->cpus_allowed, parent_cs->cpus_allowed);
 	return;
 }
 
@@ -1799,6 +1811,10 @@ static struct cgroup_subsys_state *cpuset_create(
 	cs = kmalloc(sizeof(*cs), GFP_KERNEL);
 	if (!cs)
 		return ERR_PTR(-ENOMEM);
+	if (!alloc_cpumask_var(&cs->cpus_allowed, GFP_KERNEL)) {
+		kfree(cs);
+		return ERR_PTR(-ENOMEM);
+	}
 
 	cpuset_update_task_memory_state();
 	cs->flags = 0;
@@ -1807,7 +1823,7 @@ static struct cgroup_subsys_state *cpuset_create(
 	if (is_spread_slab(parent))
 		set_bit(CS_SPREAD_SLAB, &cs->flags);
 	set_bit(CS_SCHED_LOAD_BALANCE, &cs->flags);
-	cpus_clear(cs->cpus_allowed);
+	cpumask_clear(cs->cpus_allowed);
 	nodes_clear(cs->mems_allowed);
 	cs->mems_generation = cpuset_mems_generation++;
 	fmeter_init(&cs->fmeter);
@@ -1834,6 +1850,7 @@ static void cpuset_destroy(struct cgroup_subsys *ss, struct cgroup *cont)
 		update_flag(CS_SCHED_LOAD_BALANCE, cs, 0);
 
 	number_of_cpusets--;
+	free_cpumask_var(cs->cpus_allowed);
 	kfree(cs);
 }
 
@@ -1857,6 +1874,8 @@ struct cgroup_subsys cpuset_subsys = {
 
 int __init cpuset_init_early(void)
 {
+	alloc_bootmem_cpumask_var(&top_cpuset.cpus_allowed);
+
 	top_cpuset.mems_generation = cpuset_mems_generation++;
 	return 0;
 }
@@ -1872,7 +1891,7 @@ int __init cpuset_init(void)
 {
 	int err = 0;
 
-	cpus_setall(top_cpuset.cpus_allowed);
+	cpumask_setall(top_cpuset.cpus_allowed);
 	nodes_setall(top_cpuset.mems_allowed);
 
 	fmeter_init(&top_cpuset.fmeter);
@@ -1958,7 +1977,7 @@ static void remove_tasks_in_empty_cpuset(struct cpuset *cs)
 	 * has online cpus, so can't be empty).
 	 */
 	parent = cs->parent;
-	while (cpus_empty(parent->cpus_allowed) ||
+	while (cpumask_empty(parent->cpus_allowed) ||
 			nodes_empty(parent->mems_allowed))
 		parent = parent->parent;
 
@@ -1999,7 +2018,7 @@ static void scan_for_empty_cpusets(struct cpuset *root)
 		}
 
 		/* Continue past cpusets with all cpus, mems online */
-		if (cpus_subset(cp->cpus_allowed, cpu_online_map) &&
+		if (cpumask_subset(cp->cpus_allowed, cpu_online_mask) &&
 		    nodes_subset(cp->mems_allowed, node_states[N_HIGH_MEMORY]))
 			continue;
 
@@ -2007,13 +2026,14 @@ static void scan_for_empty_cpusets(struct cpuset *root)
 
 		/* Remove offline cpus and mems from this cpuset. */
 		mutex_lock(&callback_mutex);
-		cpus_and(cp->cpus_allowed, cp->cpus_allowed, cpu_online_map);
+		cpumask_and(cp->cpus_allowed, cp->cpus_allowed,
+			    cpu_online_mask);
 		nodes_and(cp->mems_allowed, cp->mems_allowed,
 						node_states[N_HIGH_MEMORY]);
 		mutex_unlock(&callback_mutex);
 
 		/* Move tasks from the empty cpuset to a parent */
-		if (cpus_empty(cp->cpus_allowed) ||
+		if (cpumask_empty(cp->cpus_allowed) ||
 		     nodes_empty(cp->mems_allowed))
 			remove_tasks_in_empty_cpuset(cp);
 		else {
@@ -2054,7 +2074,7 @@ static int cpuset_track_online_cpus(struct notifier_block *unused_nb,
 	}
 
 	cgroup_lock();
-	top_cpuset.cpus_allowed = cpu_online_map;
+	cpumask_copy(top_cpuset.cpus_allowed, cpu_online_mask);
 	scan_for_empty_cpusets(&top_cpuset);
 	ndoms = generate_sched_domains(&doms, &attr);
 	cgroup_unlock();
@@ -2099,7 +2119,7 @@ static int cpuset_track_online_nodes(struct notifier_block *self,
 
 void __init cpuset_init_smp(void)
 {
-	top_cpuset.cpus_allowed = cpu_online_map;
+	cpumask_copy(top_cpuset.cpus_allowed, cpu_online_mask);
 	top_cpuset.mems_allowed = node_states[N_HIGH_MEMORY];
 
 	hotcpu_notifier(cpuset_track_online_cpus, 0);
@@ -2111,7 +2131,7 @@ void __init cpuset_init_smp(void)
  * @tsk: pointer to task_struct from which to obtain cpuset->cpus_allowed.
  * @pmask: pointer to cpumask_t variable to receive cpus_allowed set.
  *
- * Description: Returns the cpumask_t cpus_allowed of the cpuset
+ * Description: Returns the cpumask_var_t cpus_allowed of the cpuset
  * attached to the specified @tsk.  Guaranteed to return some non-empty
  * subset of cpu_online_map, even if this means going outside the
  * tasks cpuset.
-- 
1.5.4.rc3

[PATCH 6/6] cpuset: remove remaining pointers to cpumask_t

[Li Zefan]

Impact: cleanups, use new cpumask API

Final trivial cleanups: mainly s/cpumask_t/struct cpumask

Note there is a FIXME in generate_sched_domains(). A future patch will
change struct cpumask *doms to struct cpumask *doms[].
(I suppose Rusty will do this.)

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 include/linux/cpuset.h |   10 ++++++----
 kernel/cpuset.c        |   28 +++++++++++++++-------------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index 51ea2bd..90c6074 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -20,8 +20,9 @@ extern int number_of_cpusets;	/* How many cpusets are defined in system? */
 extern int cpuset_init_early(void);
 extern int cpuset_init(void);
 extern void cpuset_init_smp(void);
-extern void cpuset_cpus_allowed(struct task_struct *p, cpumask_t *mask);
-extern void cpuset_cpus_allowed_locked(struct task_struct *p, cpumask_t *mask);
+extern void cpuset_cpus_allowed(struct task_struct *p, struct cpumask *mask);
+extern void cpuset_cpus_allowed_locked(struct task_struct *p,
+				       struct cpumask *mask);
 extern nodemask_t cpuset_mems_allowed(struct task_struct *p);
 #define cpuset_current_mems_allowed (current->mems_allowed)
 void cpuset_init_current_mems_allowed(void);
@@ -86,12 +87,13 @@ static inline int cpuset_init_early(void) { return 0; }
 static inline int cpuset_init(void) { return 0; }
 static inline void cpuset_init_smp(void) {}
 
-static inline void cpuset_cpus_allowed(struct task_struct *p, cpumask_t *mask)
+static inline void cpuset_cpus_allowed(struct task_struct *p,
+				       struct cpumask *mask)
 {
 	*mask = cpu_possible_map;
 }
 static inline void cpuset_cpus_allowed_locked(struct task_struct *p,
-								cpumask_t *mask)
+					      struct cpumask *mask)
 {
 	*mask = cpu_possible_map;
 }
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 25df51f..a87323b 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -289,7 +289,8 @@ static struct file_system_type cpuset_fs_type = {
  * Call with callback_mutex held.
  */
 
-static void guarantee_online_cpus(const struct cpuset *cs, cpumask_t *pmask)
+static void guarantee_online_cpus(const struct cpuset *cs,
+				  struct cpumask *pmask)
 {
 	while (cs && !cpumask_intersects(cs->cpus_allowed, cpu_online_mask))
 		cs = cs->parent;
@@ -610,7 +611,8 @@ update_domain_attr_tree(struct sched_domain_attr *dattr, struct cpuset *c)
  *	element of the partition (one sched domain) to be passed to
  *	partition_sched_domains().
  */
-static int generate_sched_domains(cpumask_t **domains,
+/* FIXME: see the FIXME in partition_sched_domains() */
+static int generate_sched_domains(struct cpumask **domains,
 			struct sched_domain_attr **attributes)
 {
 	LIST_HEAD(q);		/* queue of cpusets to be scanned */
@@ -618,10 +620,10 @@ static int generate_sched_domains(cpumask_t **domains,
 	struct cpuset **csa;	/* array of all cpuset ptrs */
 	int csn;		/* how many cpuset ptrs in csa so far */
 	int i, j, k;		/* indices for partition finding loops */
-	cpumask_t *doms;	/* resulting partition; i.e. sched domains */
+	struct cpumask *doms;	/* resulting partition; i.e. sched domains */
 	struct sched_domain_attr *dattr;  /* attributes for custom domains */
 	int ndoms = 0;		/* number of sched domains in result */
-	int nslot;		/* next empty doms[] cpumask_t slot */
+	int nslot;		/* next empty doms[] struct cpumask slot */
 
 	doms = NULL;
 	dattr = NULL;
@@ -629,7 +631,7 @@ static int generate_sched_domains(cpumask_t **domains,
 
 	/* Special case for the 99% of systems with one, full, sched domain */
 	if (is_sched_load_balance(&top_cpuset)) {
-		doms = kmalloc(sizeof(cpumask_t), GFP_KERNEL);
+		doms = kmalloc(cpumask_size(), GFP_KERNEL);
 		if (!doms)
 			goto done;
 
@@ -708,7 +710,7 @@ restart:
 	 * Now we know how many domains to create.
 	 * Convert <csn, csa> to <ndoms, doms> and populate cpu masks.
 	 */
-	doms = kmalloc(ndoms * sizeof(cpumask_t), GFP_KERNEL);
+	doms = kmalloc(ndoms * cpumask_size(), GFP_KERNEL);
 	if (!doms)
 		goto done;
 
@@ -720,7 +722,7 @@ restart:
 
 	for (nslot = 0, i = 0; i < csn; i++) {
 		struct cpuset *a = csa[i];
-		cpumask_t *dp;
+		struct cpumask *dp;
 		int apn = a->pn;
 
 		if (apn < 0) {
@@ -743,7 +745,7 @@ restart:
 			continue;
 		}
 
-		cpus_clear(*dp);
+		cpumask_clear(dp);
 		if (dattr)
 			*(dattr + nslot) = SD_ATTR_INIT;
 		for (j = i; j < csn; j++) {
@@ -790,7 +792,7 @@ done:
 static void do_rebuild_sched_domains(struct work_struct *unused)
 {
 	struct sched_domain_attr *attr;
-	cpumask_t *doms;
+	struct cpumask *doms;
 	int ndoms;
 
 	get_online_cpus();
@@ -2059,7 +2061,7 @@ static int cpuset_track_online_cpus(struct notifier_block *unused_nb,
 				unsigned long phase, void *unused_cpu)
 {
 	struct sched_domain_attr *attr;
-	cpumask_t *doms;
+	struct cpumask *doms;
 	int ndoms;
 
 	switch (phase) {
@@ -2129,7 +2131,7 @@ void __init cpuset_init_smp(void)
 /**
  * cpuset_cpus_allowed - return cpus_allowed mask from a tasks cpuset.
  * @tsk: pointer to task_struct from which to obtain cpuset->cpus_allowed.
- * @pmask: pointer to cpumask_t variable to receive cpus_allowed set.
+ * @pmask: pointer to struct cpumask variable to receive cpus_allowed set.
  *
  * Description: Returns the cpumask_var_t cpus_allowed of the cpuset
  * attached to the specified @tsk.  Guaranteed to return some non-empty
@@ -2137,7 +2139,7 @@ void __init cpuset_init_smp(void)
  * tasks cpuset.
  **/
 
-void cpuset_cpus_allowed(struct task_struct *tsk, cpumask_t *pmask)
+void cpuset_cpus_allowed(struct task_struct *tsk, struct cpumask *pmask)
 {
 	mutex_lock(&callback_mutex);
 	cpuset_cpus_allowed_locked(tsk, pmask);
@@ -2148,7 +2150,7 @@ void cpuset_cpus_allowed(struct task_struct *tsk, cpumask_t *pmask)
  * cpuset_cpus_allowed_locked - return cpus_allowed mask from a tasks cpuset.
  * Must be called with callback_mutex held.
  **/
-void cpuset_cpus_allowed_locked(struct task_struct *tsk, cpumask_t *pmask)
+void cpuset_cpus_allowed_locked(struct task_struct *tsk, struct cpumask *pmask)
 {
 	task_lock(tsk);
 	guarantee_online_cpus(task_cs(tsk), pmask);
-- 
1.5.4.rc3

Re: [PATCH 0/6] cpuset: convert to new cpumask API

[Mike Travis]

Li Zefan wrote:
> This patchset converts cpuset to use new cpumask API, and thus
> remove on stack cpumask_t to reduce stack usage.
> 
> Before:
>  # cat kernel/cpuset.c include/linux/cpuset.h | grep -c cpumask_t
>  21
> After:
>  # cat kernel/cpuset.c include/linux/cpuset.h | grep -c cpumask_t
>  0
> 
> The patchset is based on mmotm 2008-12-30-16-05.
> 
> [PATCH 1/6] cpuset: remove on stack cpumask_t in cpuset_sprintf_cpulist()
> [PATCH 2/6] cpuset: remove on stack cpumask_t in cpuset_can_attach()
> [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t
> [PATCH 4/6] cpuset: don't allocate trial cpuset on stack
> [PATCH 5/6] cpuset: convert cpuset->cpus_allowed to cpumask_var_t
> [PATCH 6/6] cpuset: remove remaining pointers to cpumask_t
> ---
>  include/linux/cpuset.h   |   10 +
>  kernel/cpuset.c          |  279 ++++++++++++++++++++++++++++-------------------
>  3 files changed, 174 insertions(+), 115 deletions(-)
> 


These all look good Li!  Thanks for doing this.  You can add my:

Acked-by: Mike Travis <travis@sgi.com>

Re: [PATCH 0/6] cpuset: convert to new cpumask API

[Rusty Russell]

On Wednesday 31 December 2008 19:04:46 Li Zefan wrote:
> This patchset converts cpuset to use new cpumask API, and thus
> remove on stack cpumask_t to reduce stack usage.
> 
> Before:
>  # cat kernel/cpuset.c include/linux/cpuset.h | grep -c cpumask_t
>  21
> After:
>  # cat kernel/cpuset.c include/linux/cpuset.h | grep -c cpumask_t
>  0
> 
> The patchset is based on mmotm 2008-12-30-16-05.

Wow, thanks!

Rusty.

Re: [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

[Andrew Morton]

On Wed, 31 Dec 2008 16:36:11 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:

> Impact: reduce stack usage
> 
> Allocate a cpumask_var_t in cpuset_can_attach() and then use it in
> cpuset_attach(), so we won't fail cpuset_attach().
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
>  kernel/cpuset.c |   31 +++++++++++++++++++++++++------
>  1 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index afa29cf..b4c36d5 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -1306,6 +1306,14 @@ static int fmeter_getrate(struct fmeter *fmp)
>  	return val;
>  }
>  
> +/*
> + * Since cpuset_attach() can't fail, we allocate a cpumask_var_t in
> + * cpuset_can_attach() and then use it in cpuset_attach().
> + *
> + * Protected by cgroup_lock.

OK, but it's a bit fragile.

How are we to ensure that all calls to cpuset_can_attach() are followed
by a call to cpuset_attach(), and how are we to ensure that both calls
are protected by the same taking of the lock?  For all time?

In fact, it's broken right now, isn't it?  If cgroup_attach_task()'s
call to find_css_set() fails, we leak the result of cpuset_can_attach()'s
alloc_cpumask_var()?

Re: [PATCH 4/6] cpuset: don't allocate trial cpuset on stack

[Andrew Morton]

On Wed, 31 Dec 2008 16:36:41 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:

> Impact: cleanups, reduce stack usage
> 
> This patch prepares for the next patch. When we convert cpuset.cpus_allowed
> to cpumask_var_t, (trialcs = *cs) no longer works.
> 
> Another result of this patch is reducing stack usage of trialcs. sizeof(*cs)
> can be as large as 148 bytes on x86_64, so it's really not good to have it
> on stack.
> 

err, what?

> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -415,6 +415,24 @@ static int is_cpuset_subset(const struct cpuset *p, const struct cpuset *q)
>  		is_mem_exclusive(p) <= is_mem_exclusive(q);
>  }
>  
> +/**
> + * alloc_trial_cpuset - allocate a trial cpuset
> + * @cs: the cpuset that the trial cpuset duplicates
> + */
> +static struct cpuset *alloc_trial_cpuset(const struct cpuset *cs)
> +{
> +	return kmemdup(cs, sizeof(*cs), GFP_KERNEL);
> +}

We copy a cpuset by value?

> -static int update_cpumask(struct cpuset *cs, const char *buf)
> +static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
> +			  const char *buf)
>  {
>  	struct ptr_heap heap;
> -	struct cpuset trialcs;
>  	int retval;
>  	int is_load_balanced;
>  
> @@ -891,8 +909,6 @@ static int update_cpumask(struct cpuset *cs, const char *buf)
>  	if (cs == &top_cpuset)
>  		return -EACCES;
>  
> -	trialcs = *cs;

Yes, we already do.

That thing contains spinlocks and list_heads (at least), which cannot
be copied in this way.

Seems that we're doing this gross thing because it just so happens that
we only use the cpus_allowed and mems_allowed fields, and because
several of the called functions require a cpuset*, but only needed a
cpumask_t.

How perfectly beastly.

Re: [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

[Li Zefan]

Andrew Morton wrote:
> On Wed, 31 Dec 2008 16:36:11 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:
> 
>> Impact: reduce stack usage
>>
>> Allocate a cpumask_var_t in cpuset_can_attach() and then use it in
>> cpuset_attach(), so we won't fail cpuset_attach().
>>
>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>> ---
>>  kernel/cpuset.c |   31 +++++++++++++++++++++++++------
>>  1 files changed, 25 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
>> index afa29cf..b4c36d5 100644
>> --- a/kernel/cpuset.c
>> +++ b/kernel/cpuset.c
>> @@ -1306,6 +1306,14 @@ static int fmeter_getrate(struct fmeter *fmp)
>>  	return val;
>>  }
>>  
>> +/*
>> + * Since cpuset_attach() can't fail, we allocate a cpumask_var_t in
>> + * cpuset_can_attach() and then use it in cpuset_attach().
>> + *
>> + * Protected by cgroup_lock.
> 
> OK, but it's a bit fragile.
> 
> How are we to ensure that all calls to cpuset_can_attach() are followed
> by a call to cpuset_attach()

my fault :(

> and how are we to ensure that both calls
> are protected by the same taking of the lock?  For all time?
> 

It's insured by cgroup. The 2 functions are callbacks that will be called
by cgroup core only.

> In fact, it's broken right now, isn't it?  If cgroup_attach_task()'s
> call to find_css_set() fails, we leak the result of cpuset_can_attach()'s
> alloc_cpumask_var()?
> 

Below is a replacement:

=================

[PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

Allocate a global cpumask_var_t at boot, and use it in cpuset_attach(), so
we won't fail cpuset_attach().

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Mike Travis <travis@sgi.com>
---
 kernel/cpuset.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index afa29cf..1e32e6b 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -1306,6 +1306,9 @@ static int fmeter_getrate(struct fmeter *fmp)
 	return val;
 }
 
+/* Protected by cgroup_lock */
+static cpumask_var_t cpus_attach;
+
 /* Called by cgroups to determine if a cpuset is usable; cgroup_mutex held */
 static int cpuset_can_attach(struct cgroup_subsys *ss,
 			     struct cgroup *cont, struct task_struct *tsk)
@@ -1330,7 +1333,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
 			  struct cgroup *cont, struct cgroup *oldcont,
 			  struct task_struct *tsk)
 {
-	cpumask_t cpus;
 	nodemask_t from, to;
 	struct mm_struct *mm;
 	struct cpuset *cs = cgroup_cs(cont);
@@ -1338,13 +1340,13 @@ static void cpuset_attach(struct cgroup_subsys *ss,
 	int err;
 
 	if (cs == &top_cpuset) {
-		cpus = cpu_possible_map;
+		cpumask_copy(cpus_attach, cpu_possible_mask);
 	} else {
 		mutex_lock(&callback_mutex);
-		guarantee_online_cpus(cs, &cpus);
+		guarantee_online_cpus(cs, cpus_attach);
 		mutex_unlock(&callback_mutex);
 	}
-	err = set_cpus_allowed_ptr(tsk, &cpus);
+	err = set_cpus_allowed_ptr(tsk, cpus_attach);
 	if (err)
 		return;
 
@@ -1357,7 +1359,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
 			cpuset_migrate_mm(mm, &from, &to);
 		mmput(mm);
 	}
-
 }
 
 /* The various types of files and directories in a cpuset file system */
@@ -1838,6 +1839,9 @@ int __init cpuset_init(void)
 	if (err < 0)
 		return err;
 
+	if (!alloc_cpumask_var(&cpus_attach, GFP_KERNEL))
+		BUG();
+
 	number_of_cpusets = 1;
 	return 0;
 }
-- 
1.5.4.rc3

Re: [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

[Andrew Morton]

On Mon, 05 Jan 2009 16:47:21 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:

> Allocate a global cpumask_var_t at boot, and use it in cpuset_attach(), so
> we won't fail cpuset_attach().
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> Acked-by: Mike Travis <travis@sgi.com>
> ---
>  kernel/cpuset.c |   14 +++++++++-----
>  1 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index afa29cf..1e32e6b 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -1306,6 +1306,9 @@ static int fmeter_getrate(struct fmeter *fmp)
>  	return val;
>  }
>  
> +/* Protected by cgroup_lock */
> +static cpumask_var_t cpus_attach;
> +
>  /* Called by cgroups to determine if a cpuset is usable; cgroup_mutex held */
>  static int cpuset_can_attach(struct cgroup_subsys *ss,
>  			     struct cgroup *cont, struct task_struct *tsk)
> @@ -1330,7 +1333,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
>  			  struct cgroup *cont, struct cgroup *oldcont,
>  			  struct task_struct *tsk)
>  {
> -	cpumask_t cpus;
>  	nodemask_t from, to;
>  	struct mm_struct *mm;
>  	struct cpuset *cs = cgroup_cs(cont);
> @@ -1338,13 +1340,13 @@ static void cpuset_attach(struct cgroup_subsys *ss,
>  	int err;
>  
>  	if (cs == &top_cpuset) {
> -		cpus = cpu_possible_map;
> +		cpumask_copy(cpus_attach, cpu_possible_mask);
>  	} else {
>  		mutex_lock(&callback_mutex);
> -		guarantee_online_cpus(cs, &cpus);
> +		guarantee_online_cpus(cs, cpus_attach);
>  		mutex_unlock(&callback_mutex);
>  	}
> -	err = set_cpus_allowed_ptr(tsk, &cpus);
> +	err = set_cpus_allowed_ptr(tsk, cpus_attach);
>  	if (err)
>  		return;
>  
> @@ -1357,7 +1359,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
>  			cpuset_migrate_mm(mm, &from, &to);
>  		mmput(mm);
>  	}
> -
>  }
>  
>  /* The various types of files and directories in a cpuset file system */
> @@ -1838,6 +1839,9 @@ int __init cpuset_init(void)
>  	if (err < 0)
>  		return err;
>  
> +	if (!alloc_cpumask_var(&cpus_attach, GFP_KERNEL))
> +		BUG();
> +
>  	number_of_cpusets = 1;
>  	return 0;
>  }

OK, that works.

Do we need to dynamically allocate cpus_attach?  Can we just do

static cpumask_t cpus_attach;

?

Re: [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

[Li Zefan]

Andrew Morton wrote:
> On Mon, 05 Jan 2009 16:47:21 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:
> 
>> Allocate a global cpumask_var_t at boot, and use it in cpuset_attach(), so
>> we won't fail cpuset_attach().
>>
>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>> Acked-by: Mike Travis <travis@sgi.com>
>> ---
>>  kernel/cpuset.c |   14 +++++++++-----
>>  1 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
>> index afa29cf..1e32e6b 100644
>> --- a/kernel/cpuset.c
>> +++ b/kernel/cpuset.c
>> @@ -1306,6 +1306,9 @@ static int fmeter_getrate(struct fmeter *fmp)
>>  	return val;
>>  }
>>  
>> +/* Protected by cgroup_lock */
>> +static cpumask_var_t cpus_attach;
>> +
>>  /* Called by cgroups to determine if a cpuset is usable; cgroup_mutex held */
>>  static int cpuset_can_attach(struct cgroup_subsys *ss,
>>  			     struct cgroup *cont, struct task_struct *tsk)
>> @@ -1330,7 +1333,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
>>  			  struct cgroup *cont, struct cgroup *oldcont,
>>  			  struct task_struct *tsk)
>>  {
>> -	cpumask_t cpus;
>>  	nodemask_t from, to;
>>  	struct mm_struct *mm;
>>  	struct cpuset *cs = cgroup_cs(cont);
>> @@ -1338,13 +1340,13 @@ static void cpuset_attach(struct cgroup_subsys *ss,
>>  	int err;
>>  
>>  	if (cs == &top_cpuset) {
>> -		cpus = cpu_possible_map;
>> +		cpumask_copy(cpus_attach, cpu_possible_mask);
>>  	} else {
>>  		mutex_lock(&callback_mutex);
>> -		guarantee_online_cpus(cs, &cpus);
>> +		guarantee_online_cpus(cs, cpus_attach);
>>  		mutex_unlock(&callback_mutex);
>>  	}
>> -	err = set_cpus_allowed_ptr(tsk, &cpus);
>> +	err = set_cpus_allowed_ptr(tsk, cpus_attach);
>>  	if (err)
>>  		return;
>>  
>> @@ -1357,7 +1359,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
>>  			cpuset_migrate_mm(mm, &from, &to);
>>  		mmput(mm);
>>  	}
>> -
>>  }
>>  
>>  /* The various types of files and directories in a cpuset file system */
>> @@ -1838,6 +1839,9 @@ int __init cpuset_init(void)
>>  	if (err < 0)
>>  		return err;
>>  
>> +	if (!alloc_cpumask_var(&cpus_attach, GFP_KERNEL))
>> +		BUG();
>> +
>>  	number_of_cpusets = 1;
>>  	return 0;
>>  }
> 
> OK, that works.
> 
> Do we need to dynamically allocate cpus_attach?  Can we just do
> 
> static cpumask_t cpus_attach;
> 
> ?
> 

Yes, it's used by cpuset_attach() only, and cpuset_attach() is called with
cgroup_lock() held, so it won't happen that 2 threads access cpus_attach
concurrently.

Re: [PATCH 4/6] cpuset: don't allocate trial cpuset on stack

[Li Zefan]

>> -static int update_cpumask(struct cpuset *cs, const char *buf)
>> +static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>> +			  const char *buf)
>>  {
>>  	struct ptr_heap heap;
>> -	struct cpuset trialcs;
>>  	int retval;
>>  	int is_load_balanced;
>>  
>> @@ -891,8 +909,6 @@ static int update_cpumask(struct cpuset *cs, const char *buf)
>>  	if (cs == &top_cpuset)
>>  		return -EACCES;
>>  
>> -	trialcs = *cs;
> 
> Yes, we already do.
> 
> That thing contains spinlocks and list_heads (at least), which cannot
> be copied in this way.
> 
> Seems that we're doing this gross thing because it just so happens that
> we only use the cpus_allowed and mems_allowed fields, and because

and also cpuset->flags.

> several of the called functions require a cpuset*, but only needed a
> cpumask_t.
> 
> How perfectly beastly.
> 

trial cpuset is used to ease the validation of changes to a cpuset. It
can be get rid of, but I'm afraid that the resulting code will be more
complex and less readable.

Re: [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

[Andrew Morton]

On Mon, 05 Jan 2009 17:04:54 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:

> Andrew Morton wrote:
> > On Mon, 05 Jan 2009 16:47:21 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:
> > 
> >> Allocate a global cpumask_var_t at boot, and use it in cpuset_attach(), so
> >> we won't fail cpuset_attach().
> >>
> >> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> >> Acked-by: Mike Travis <travis@sgi.com>
> >> ---
> >>  kernel/cpuset.c |   14 +++++++++-----
> >>  1 files changed, 9 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> >> index afa29cf..1e32e6b 100644
> >> --- a/kernel/cpuset.c
> >> +++ b/kernel/cpuset.c
> >> @@ -1306,6 +1306,9 @@ static int fmeter_getrate(struct fmeter *fmp)
> >>  	return val;
> >>  }
> >>  
> >> +/* Protected by cgroup_lock */
> >> +static cpumask_var_t cpus_attach;
> >> +
> >>  /* Called by cgroups to determine if a cpuset is usable; cgroup_mutex held */
> >>  static int cpuset_can_attach(struct cgroup_subsys *ss,
> >>  			     struct cgroup *cont, struct task_struct *tsk)
> >> @@ -1330,7 +1333,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
> >>  			  struct cgroup *cont, struct cgroup *oldcont,
> >>  			  struct task_struct *tsk)
> >>  {
> >> -	cpumask_t cpus;
> >>  	nodemask_t from, to;
> >>  	struct mm_struct *mm;
> >>  	struct cpuset *cs = cgroup_cs(cont);
> >> @@ -1338,13 +1340,13 @@ static void cpuset_attach(struct cgroup_subsys *ss,
> >>  	int err;
> >>  
> >>  	if (cs == &top_cpuset) {
> >> -		cpus = cpu_possible_map;
> >> +		cpumask_copy(cpus_attach, cpu_possible_mask);
> >>  	} else {
> >>  		mutex_lock(&callback_mutex);
> >> -		guarantee_online_cpus(cs, &cpus);
> >> +		guarantee_online_cpus(cs, cpus_attach);
> >>  		mutex_unlock(&callback_mutex);
> >>  	}
> >> -	err = set_cpus_allowed_ptr(tsk, &cpus);
> >> +	err = set_cpus_allowed_ptr(tsk, cpus_attach);
> >>  	if (err)
> >>  		return;
> >>  
> >> @@ -1357,7 +1359,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
> >>  			cpuset_migrate_mm(mm, &from, &to);
> >>  		mmput(mm);
> >>  	}
> >> -
> >>  }
> >>  
> >>  /* The various types of files and directories in a cpuset file system */
> >> @@ -1838,6 +1839,9 @@ int __init cpuset_init(void)
> >>  	if (err < 0)
> >>  		return err;
> >>  
> >> +	if (!alloc_cpumask_var(&cpus_attach, GFP_KERNEL))
> >> +		BUG();
> >> +
> >>  	number_of_cpusets = 1;
> >>  	return 0;
> >>  }
> > 
> > OK, that works.
> > 
> > Do we need to dynamically allocate cpus_attach?  Can we just do
> > 
> > static cpumask_t cpus_attach;
> > 
> > ?
> > 
> 
> Yes, it's used by cpuset_attach() only, and cpuset_attach() is called with
> cgroup_lock() held, so it won't happen that 2 threads access cpus_attach
> concurrently.

You misunderstand my question.  I think.

Can we allocate cpus_attach at compile time?  Completely, not
partially.  By doing

static cpumask_t cpus_attach;

instead of

static cpumask_var_t cpus_attach;
...
	alloc_cpumask_var(&cpus_attach, GFP_KERNEL);

?

Re: [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

[Li Zefan]

>>> OK, that works.
>>>
>>> Do we need to dynamically allocate cpus_attach?  Can we just do
>>>
>>> static cpumask_t cpus_attach;
>>>
>>> ?
>>>
>> Yes, it's used by cpuset_attach() only, and cpuset_attach() is called with
>> cgroup_lock() held, so it won't happen that 2 threads access cpus_attach
>> concurrently.
> 
> You misunderstand my question.  I think.
> 
> Can we allocate cpus_attach at compile time?  Completely, not
> partially.  By doing
> 
> static cpumask_t cpus_attach;
> 
> instead of
> 
> static cpumask_var_t cpus_attach;
> ...
> 	alloc_cpumask_var(&cpus_attach, GFP_KERNEL);
> 
> ?

Ah, I misunderstood. Yes a static cpumask_t works, but what Mike Travis and
Rusty is doing is to remove cpumask_t completely, and replace cpumask_t
with cpumask_var_t wherever possible.

Re: [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

[Rusty Russell]

On Monday 05 January 2009 19:31:47 Andrew Morton wrote:
> On Mon, 05 Jan 2009 16:47:21 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:
> 
> > Allocate a global cpumask_var_t at boot, and use it in cpuset_attach(), so
> > we won't fail cpuset_attach().
> > 
> > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> > Acked-by: Mike Travis <travis@sgi.com>
> > ---
> >  kernel/cpuset.c |   14 +++++++++-----
> >  1 files changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> > index afa29cf..1e32e6b 100644
> > --- a/kernel/cpuset.c
> > +++ b/kernel/cpuset.c
> > @@ -1306,6 +1306,9 @@ static int fmeter_getrate(struct fmeter *fmp)
> >  	return val;
> >  }
> >  
> > +/* Protected by cgroup_lock */
> > +static cpumask_var_t cpus_attach;
> > +
> >  /* Called by cgroups to determine if a cpuset is usable; cgroup_mutex held */
> >  static int cpuset_can_attach(struct cgroup_subsys *ss,
> >  			     struct cgroup *cont, struct task_struct *tsk)
> > @@ -1330,7 +1333,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
> >  			  struct cgroup *cont, struct cgroup *oldcont,
> >  			  struct task_struct *tsk)
> >  {
> > -	cpumask_t cpus;
> >  	nodemask_t from, to;
> >  	struct mm_struct *mm;
> >  	struct cpuset *cs = cgroup_cs(cont);
> > @@ -1338,13 +1340,13 @@ static void cpuset_attach(struct cgroup_subsys *ss,
> >  	int err;
> >  
> >  	if (cs == &top_cpuset) {
> > -		cpus = cpu_possible_map;
> > +		cpumask_copy(cpus_attach, cpu_possible_mask);
> >  	} else {
> >  		mutex_lock(&callback_mutex);
> > -		guarantee_online_cpus(cs, &cpus);
> > +		guarantee_online_cpus(cs, cpus_attach);
> >  		mutex_unlock(&callback_mutex);
> >  	}
> > -	err = set_cpus_allowed_ptr(tsk, &cpus);
> > +	err = set_cpus_allowed_ptr(tsk, cpus_attach);
> >  	if (err)
> >  		return;
> >  
> > @@ -1357,7 +1359,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
> >  			cpuset_migrate_mm(mm, &from, &to);
> >  		mmput(mm);
> >  	}
> > -
> >  }
> >  
> >  /* The various types of files and directories in a cpuset file system */
> > @@ -1838,6 +1839,9 @@ int __init cpuset_init(void)
> >  	if (err < 0)
> >  		return err;
> >  
> > +	if (!alloc_cpumask_var(&cpus_attach, GFP_KERNEL))
> > +		BUG();
> > +
> >  	number_of_cpusets = 1;
> >  	return 0;
> >  }
> 
> OK, that works.
> 
> Do we need to dynamically allocate cpus_attach?  Can we just do
> 
> static cpumask_t cpus_attach;

Well, (1) we're deprecating the typedef, (2) we really want the NR_CPUS=16384
and nr_cpu_ids=4 case not to suck and this sets a bad example, but moreover
(3) struct cpumask won't be defined eventually when CONFIG_CPUMASKS_OFFSTACK=y
so that won't compile.

Cheers,
Rusty.

Re: [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t

[Paul Menage]

On Mon, Jan 5, 2009 at 12:47 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
>
> [PATCH 3/6] cpuset: convert cpuset_attach() to use cpumask_var_t
>
> Allocate a global cpumask_var_t at boot, and use it in cpuset_attach(), so
> we won't fail cpuset_attach().
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> Acked-by: Mike Travis <travis@sgi.com>

OK, this version looks fine.

Acked-by: Paul Menage <menage@google.com>

> ---
>  kernel/cpuset.c |   14 +++++++++-----
>  1 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index afa29cf..1e32e6b 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -1306,6 +1306,9 @@ static int fmeter_getrate(struct fmeter *fmp)
>        return val;
>  }
>
> +/* Protected by cgroup_lock */
> +static cpumask_var_t cpus_attach;
> +
>  /* Called by cgroups to determine if a cpuset is usable; cgroup_mutex held */
>  static int cpuset_can_attach(struct cgroup_subsys *ss,
>                             struct cgroup *cont, struct task_struct *tsk)
> @@ -1330,7 +1333,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
>                          struct cgroup *cont, struct cgroup *oldcont,
>                          struct task_struct *tsk)
>  {
> -       cpumask_t cpus;
>        nodemask_t from, to;
>        struct mm_struct *mm;
>        struct cpuset *cs = cgroup_cs(cont);
> @@ -1338,13 +1340,13 @@ static void cpuset_attach(struct cgroup_subsys *ss,
>        int err;
>
>        if (cs == &top_cpuset) {
> -               cpus = cpu_possible_map;
> +               cpumask_copy(cpus_attach, cpu_possible_mask);
>        } else {
>                mutex_lock(&callback_mutex);
> -               guarantee_online_cpus(cs, &cpus);
> +               guarantee_online_cpus(cs, cpus_attach);
>                mutex_unlock(&callback_mutex);
>        }
> -       err = set_cpus_allowed_ptr(tsk, &cpus);
> +       err = set_cpus_allowed_ptr(tsk, cpus_attach);
>        if (err)
>                return;
>
> @@ -1357,7 +1359,6 @@ static void cpuset_attach(struct cgroup_subsys *ss,
>                        cpuset_migrate_mm(mm, &from, &to);
>                mmput(mm);
>        }
> -
>  }
>
>  /* The various types of files and directories in a cpuset file system */
> @@ -1838,6 +1839,9 @@ int __init cpuset_init(void)
>        if (err < 0)
>                return err;
>
> +       if (!alloc_cpumask_var(&cpus_attach, GFP_KERNEL))
> +               BUG();
> +
>        number_of_cpusets = 1;
>        return 0;
>  }
> --
> 1.5.4.rc3
>
>
>