[PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Miao Xie]

extract two functions from update_cpumask() and update_nodemask().They will be
used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
offline/online.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
Acked-by: Paul Jackson <pj@sgi.com>

---
 kernel/cpuset.c |  179 +++++++++++++++++++++++++++++++++----------------------
 1 files changed, 108 insertions(+), 71 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 86ea9e3..dc16f71 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -766,6 +766,37 @@ static void cpuset_change_cpumask(struct task_struct *tsk,
 }
 
 /**
+ * update_tasks_cpumask - Update the cpumasks of tasks in the cpuset.
+ * @cs: the cpuset in which each task's cpus_allowed mask needs to be changed
+ *
+ * Called with cgroup_mutex held
+ *
+ * The cgroup_scan_tasks() function will scan all the tasks in a cgroup,
+ * calling callback functions for each.
+ *
+ * Return 0 if successful, -errno if not.
+ */
+static int update_tasks_cpumask(struct cpuset *cs)
+{
+	struct cgroup_scanner scan;
+	struct ptr_heap heap;
+	int retval;
+
+	retval = heap_init(&heap, PAGE_SIZE, GFP_KERNEL, &started_after);
+	if (retval)
+		return retval;
+
+	scan.cg = cs->css.cgroup;
+	scan.test_task = cpuset_test_cpumask;
+	scan.process_task = cpuset_change_cpumask;
+	scan.heap = &heap;
+	retval = cgroup_scan_tasks(&scan);
+
+	heap_free(&heap);
+	return retval;
+}
+
+/**
  * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it
  * @cs: the cpuset to consider
  * @buf: buffer of cpu numbers written to this cpuset
@@ -773,8 +804,6 @@ static void cpuset_change_cpumask(struct task_struct *tsk,
 static int update_cpumask(struct cpuset *cs, char *buf)
 {
 	struct cpuset trialcs;
-	struct cgroup_scanner scan;
-	struct ptr_heap heap;
 	int retval;
 	int is_load_balanced;
 
@@ -807,10 +836,6 @@ static int update_cpumask(struct cpuset *cs, char *buf)
 	if (cpus_equal(cs->cpus_allowed, trialcs.cpus_allowed))
 		return 0;
 
-	retval = heap_init(&heap, PAGE_SIZE, GFP_KERNEL, &started_after);
-	if (retval)
-		return retval;
-
 	is_load_balanced = is_sched_load_balance(&trialcs);
 
 	mutex_lock(&callback_mutex);
@@ -821,12 +846,9 @@ static int update_cpumask(struct cpuset *cs, char *buf)
 	 * Scan tasks in the cpuset, and update the cpumasks of any
 	 * that need an update.
 	 */
-	scan.cg = cs->css.cgroup;
-	scan.test_task = cpuset_test_cpumask;
-	scan.process_task = cpuset_change_cpumask;
-	scan.heap = &heap;
-	cgroup_scan_tasks(&scan);
-	heap_free(&heap);
+	retval = update_tasks_cpumask(cs);
+	if (retval < 0)
+		return retval;
 
 	if (is_load_balanced)
 		rebuild_sched_domains();
@@ -882,72 +904,25 @@ static void cpuset_migrate_mm(struct mm_struct *mm, const nodemask_t *from,
 	mutex_unlock(&callback_mutex);
 }
 
-/*
- * Handle user request to change the 'mems' memory placement
- * of a cpuset.  Needs to validate the request, update the
- * cpusets mems_allowed and mems_generation, and for each
- * task in the cpuset, rebind any vma mempolicies and if
- * the cpuset is marked 'memory_migrate', migrate the tasks
- * pages to the new memory.
- *
- * Call with cgroup_mutex held.  May take callback_mutex during call.
- * Will take tasklist_lock, scan tasklist for tasks in cpuset cs,
- * lock each such tasks mm->mmap_sem, scan its vma's and rebind
- * their mempolicies to the cpusets new mems_allowed.
- */
-
 static void *cpuset_being_rebound;
 
-static int update_nodemask(struct cpuset *cs, char *buf)
+/**
+ * update_tasks_nodemask - Update the nodemasks of tasks in the cpuset.
+ * @cs: the cpuset in which each task's mems_allowed mask needs to be changed
+ * @oldmem: old mems_allowed of cpuset cs
+ *
+ * Called with cgroup_mutex held
+ * Return 0 if successful, -errno if not.
+ */
+static int update_tasks_nodemask(struct cpuset *cs, const nodemask_t *oldmem)
 {
-	struct cpuset trialcs;
-	nodemask_t oldmem;
 	struct task_struct *p;
 	struct mm_struct **mmarray;
 	int i, n, ntasks;
 	int migrate;
 	int fudge;
-	int retval;
 	struct cgroup_iter it;
-
-	/*
-	 * top_cpuset.mems_allowed tracks node_stats[N_HIGH_MEMORY];
-	 * it's read-only
-	 */
-	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
-	 * that parsing.  The validate_change() call ensures that cpusets
-	 * with tasks have memory.
-	 */
-	buf = strstrip(buf);
-	if (!*buf) {
-		nodes_clear(trialcs.mems_allowed);
-	} else {
-		retval = nodelist_parse(buf, trialcs.mems_allowed);
-		if (retval < 0)
-			goto done;
-	}
-	nodes_and(trialcs.mems_allowed, trialcs.mems_allowed,
-						node_states[N_HIGH_MEMORY]);
-	oldmem = cs->mems_allowed;
-	if (nodes_equal(oldmem, trialcs.mems_allowed)) {
-		retval = 0;		/* Too easy - nothing to do */
-		goto done;
-	}
-	retval = validate_change(cs, &trialcs);
-	if (retval < 0)
-		goto done;
-
-	mutex_lock(&callback_mutex);
-	cs->mems_allowed = trialcs.mems_allowed;
-	cs->mems_generation = cpuset_mems_generation++;
-	mutex_unlock(&callback_mutex);
+	int retval;
 
 	cpuset_being_rebound = cs;		/* causes mpol_dup() rebind */
 
@@ -1014,7 +989,7 @@ static int update_nodemask(struct cpuset *cs, char *buf)
 
 		mpol_rebind_mm(mm, &cs->mems_allowed);
 		if (migrate)
-			cpuset_migrate_mm(mm, &oldmem, &cs->mems_allowed);
+			cpuset_migrate_mm(mm, oldmem, &cs->mems_allowed);
 		mmput(mm);
 	}
 
@@ -1026,6 +1001,69 @@ done:
 	return retval;
 }
 
+/*
+ * Handle user request to change the 'mems' memory placement
+ * of a cpuset.  Needs to validate the request, update the
+ * cpusets mems_allowed and mems_generation, and for each
+ * task in the cpuset, rebind any vma mempolicies and if
+ * the cpuset is marked 'memory_migrate', migrate the tasks
+ * pages to the new memory.
+ *
+ * Call with cgroup_mutex held.  May take callback_mutex during call.
+ * Will take tasklist_lock, scan tasklist for tasks in cpuset cs,
+ * 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, char *buf)
+{
+	struct cpuset trialcs;
+	nodemask_t oldmem;
+	int retval;
+
+	/*
+	 * top_cpuset.mems_allowed tracks node_stats[N_HIGH_MEMORY];
+	 * it's read-only
+	 */
+	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
+	 * that parsing.  The validate_change() call ensures that cpusets
+	 * with tasks have memory.
+	 */
+	buf = strstrip(buf);
+	if (!*buf) {
+		nodes_clear(trialcs.mems_allowed);
+	} else {
+		retval = nodelist_parse(buf, trialcs.mems_allowed);
+		if (retval < 0)
+			goto done;
+	}
+	nodes_and(trialcs.mems_allowed, trialcs.mems_allowed,
+						node_states[N_HIGH_MEMORY]);
+	oldmem = cs->mems_allowed;
+	if (nodes_equal(oldmem, trialcs.mems_allowed)) {
+		retval = 0;		/* Too easy - nothing to do */
+		goto done;
+	}
+	retval = validate_change(cs, &trialcs);
+	if (retval < 0)
+		goto done;
+
+	mutex_lock(&callback_mutex);
+	cs->mems_allowed = trialcs.mems_allowed;
+	cs->mems_generation = cpuset_mems_generation++;
+	mutex_unlock(&callback_mutex);
+
+	retval = update_tasks_nodemask(cs, &oldmem);
+done:
+	return retval;
+}
+
 int current_cpuset_is_being_rebound(void)
 {
 	return task_cs(current) == cpuset_being_rebound;
@@ -1938,7 +1976,6 @@ 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.
-- 
1.5.4.rc3

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Paul Jackson]

Good work, Miao.

Andrew - these two patches of Miao should be ready for you to consider now.

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

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Andrew Morton]

On Mon, 02 Jun 2008 16:31:05 +0800
Miao Xie <miaox@cn.fujitsu.com> wrote:

> extract two functions from update_cpumask() and update_nodemask().They will be
> used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
> offline/online.

Unfortunately this patch conflicts with
http://userweb.kernel.org/~akpm/mmotm/broken-out/cpusets-fix-bug-when-adding-nonexistent-cpu-or-mem.patch

Please check that the patch which I merged still makes sense.  ie: that
we did not revert the effects of that bugfix.

Thanks.

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Li Zefan]

Andrew Morton wrote:
> On Mon, 02 Jun 2008 16:31:05 +0800
> Miao Xie <miaox@cn.fujitsu.com> wrote:
> 
>> extract two functions from update_cpumask() and update_nodemask().They will be
>> used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
>> offline/online.
> 
> Unfortunately this patch conflicts with
> http://userweb.kernel.org/~akpm/mmotm/broken-out/cpusets-fix-bug-when-adding-nonexistent-cpu-or-mem.patch
> 
> Please check that the patch which I merged still makes sense.  ie: that
> we did not revert the effects of that bugfix.
> 

They are fixing 2 different bugs, so they are irrelated, but unfortunately conflict in
the code due to Miao's first patch which does code restructuring.

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Miao Xie]

on 2008-6-4 8:55 Li Zefan wrote:
> Andrew Morton wrote:
>> On Mon, 02 Jun 2008 16:31:05 +0800
>> Miao Xie <miaox@cn.fujitsu.com> wrote:
>>
>>> extract two functions from update_cpumask() and update_nodemask().They will be
>>> used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
>>> offline/online.
>> Unfortunately this patch conflicts with
>> http://userweb.kernel.org/~akpm/mmotm/broken-out/cpusets-fix-bug-when-adding-nonexistent-cpu-or-mem.patch
>>
>> Please check that the patch which I merged still makes sense.  ie: that
>> we did not revert the effects of that bugfix.
>>
> 
> They are fixing 2 different bugs, so they are irrelated, but unfortunately conflict in
> the code due to Miao's first patch which does code restructuring.
> 

Right. I will modify my patches.

[PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Miao Xie]

extract two functions from update_cpumask() and update_nodemask().They will be
used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
offline/online.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
Acked-by: Paul Jackson <pj@sgi.com>

---
 kernel/cpuset.c |  183 +++++++++++++++++++++++++++++++++----------------------
 1 files changed, 110 insertions(+), 73 deletions(-)

diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index 039baa4..aa955ee 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -766,6 +766,37 @@ static void cpuset_change_cpumask(struct task_struct *tsk,
 }
 
 /**
+ * update_tasks_cpumask - Update the cpumasks of tasks in the cpuset.
+ * @cs: the cpuset in which each task's cpus_allowed mask needs to be changed
+ *
+ * Called with cgroup_mutex held
+ *
+ * The cgroup_scan_tasks() function will scan all the tasks in a cgroup,
+ * calling callback functions for each.
+ *
+ * Return 0 if successful, -errno if not.
+ */
+static int update_tasks_cpumask(struct cpuset *cs)
+{
+	struct cgroup_scanner scan;
+	struct ptr_heap heap;
+	int retval;
+
+	retval = heap_init(&heap, PAGE_SIZE, GFP_KERNEL, &started_after);
+	if (retval)
+		return retval;
+
+	scan.cg = cs->css.cgroup;
+	scan.test_task = cpuset_test_cpumask;
+	scan.process_task = cpuset_change_cpumask;
+	scan.heap = &heap;
+	retval = cgroup_scan_tasks(&scan);
+
+	heap_free(&heap);
+	return retval;
+}
+
+/**
  * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it
  * @cs: the cpuset to consider
  * @buf: buffer of cpu numbers written to this cpuset
@@ -773,8 +804,6 @@ static void cpuset_change_cpumask(struct task_struct *tsk,
 static int update_cpumask(struct cpuset *cs, char *buf)
 {
 	struct cpuset trialcs;
-	struct cgroup_scanner scan;
-	struct ptr_heap heap;
 	int retval;
 	int is_load_balanced;
 
@@ -809,10 +838,6 @@ static int update_cpumask(struct cpuset *cs, char *buf)
 	if (cpus_equal(cs->cpus_allowed, trialcs.cpus_allowed))
 		return 0;
 
-	retval = heap_init(&heap, PAGE_SIZE, GFP_KERNEL, &started_after);
-	if (retval)
-		return retval;
-
 	is_load_balanced = is_sched_load_balance(&trialcs);
 
 	mutex_lock(&callback_mutex);
@@ -823,12 +848,9 @@ static int update_cpumask(struct cpuset *cs, char *buf)
 	 * Scan tasks in the cpuset, and update the cpumasks of any
 	 * that need an update.
 	 */
-	scan.cg = cs->css.cgroup;
-	scan.test_task = cpuset_test_cpumask;
-	scan.process_task = cpuset_change_cpumask;
-	scan.heap = &heap;
-	cgroup_scan_tasks(&scan);
-	heap_free(&heap);
+	retval = update_tasks_cpumask(cs);
+	if (retval < 0)
+		return retval;
 
 	if (is_load_balanced)
 		rebuild_sched_domains();
@@ -884,74 +906,25 @@ static void cpuset_migrate_mm(struct mm_struct *mm, const nodemask_t *from,
 	mutex_unlock(&callback_mutex);
 }
 
-/*
- * Handle user request to change the 'mems' memory placement
- * of a cpuset.  Needs to validate the request, update the
- * cpusets mems_allowed and mems_generation, and for each
- * task in the cpuset, rebind any vma mempolicies and if
- * the cpuset is marked 'memory_migrate', migrate the tasks
- * pages to the new memory.
- *
- * Call with cgroup_mutex held.  May take callback_mutex during call.
- * Will take tasklist_lock, scan tasklist for tasks in cpuset cs,
- * lock each such tasks mm->mmap_sem, scan its vma's and rebind
- * their mempolicies to the cpusets new mems_allowed.
- */
-
 static void *cpuset_being_rebound;
 
-static int update_nodemask(struct cpuset *cs, char *buf)
+/**
+ * update_tasks_nodemask - Update the nodemasks of tasks in the cpuset.
+ * @cs: the cpuset in which each task's mems_allowed mask needs to be changed
+ * @oldmem: old mems_allowed of cpuset cs
+ *
+ * Called with cgroup_mutex held
+ * Return 0 if successful, -errno if not.
+ */
+static int update_tasks_nodemask(struct cpuset *cs, const nodemask_t *oldmem)
 {
-	struct cpuset trialcs;
-	nodemask_t oldmem;
 	struct task_struct *p;
 	struct mm_struct **mmarray;
 	int i, n, ntasks;
 	int migrate;
 	int fudge;
-	int retval;
 	struct cgroup_iter it;
-
-	/*
-	 * top_cpuset.mems_allowed tracks node_stats[N_HIGH_MEMORY];
-	 * it's read-only
-	 */
-	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
-	 * that parsing.  The validate_change() call ensures that cpusets
-	 * with tasks have memory.
-	 */
-	buf = strstrip(buf);
-	if (!*buf) {
-		nodes_clear(trialcs.mems_allowed);
-	} else {
-		retval = nodelist_parse(buf, trialcs.mems_allowed);
-		if (retval < 0)
-			goto done;
-
-		if (!nodes_subset(trialcs.mems_allowed,
-				node_states[N_HIGH_MEMORY]))
-			return -EINVAL;
-	}
-	oldmem = cs->mems_allowed;
-	if (nodes_equal(oldmem, trialcs.mems_allowed)) {
-		retval = 0;		/* Too easy - nothing to do */
-		goto done;
-	}
-	retval = validate_change(cs, &trialcs);
-	if (retval < 0)
-		goto done;
-
-	mutex_lock(&callback_mutex);
-	cs->mems_allowed = trialcs.mems_allowed;
-	cs->mems_generation = cpuset_mems_generation++;
-	mutex_unlock(&callback_mutex);
+	int retval;
 
 	cpuset_being_rebound = cs;		/* causes mpol_dup() rebind */
 
@@ -1018,7 +991,7 @@ static int update_nodemask(struct cpuset *cs, char *buf)
 
 		mpol_rebind_mm(mm, &cs->mems_allowed);
 		if (migrate)
-			cpuset_migrate_mm(mm, &oldmem, &cs->mems_allowed);
+			cpuset_migrate_mm(mm, oldmem, &cs->mems_allowed);
 		mmput(mm);
 	}
 
@@ -1030,6 +1003,71 @@ done:
 	return retval;
 }
 
+/*
+ * Handle user request to change the 'mems' memory placement
+ * of a cpuset.  Needs to validate the request, update the
+ * cpusets mems_allowed and mems_generation, and for each
+ * task in the cpuset, rebind any vma mempolicies and if
+ * the cpuset is marked 'memory_migrate', migrate the tasks
+ * pages to the new memory.
+ *
+ * Call with cgroup_mutex held.  May take callback_mutex during call.
+ * Will take tasklist_lock, scan tasklist for tasks in cpuset cs,
+ * 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, char *buf)
+{
+	struct cpuset trialcs;
+	nodemask_t oldmem;
+	int retval;
+
+	/*
+	 * top_cpuset.mems_allowed tracks node_stats[N_HIGH_MEMORY];
+	 * it's read-only
+	 */
+	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
+	 * that parsing.  The validate_change() call ensures that cpusets
+	 * with tasks have memory.
+	 */
+	buf = strstrip(buf);
+	if (!*buf) {
+		nodes_clear(trialcs.mems_allowed);
+	} else {
+		retval = nodelist_parse(buf, trialcs.mems_allowed);
+		if (retval < 0)
+			goto done;
+
+		if (!nodes_subset(trialcs.mems_allowed,
+				node_states[N_HIGH_MEMORY]))
+			return -EINVAL;
+	}
+	oldmem = cs->mems_allowed;
+	if (nodes_equal(oldmem, trialcs.mems_allowed)) {
+		retval = 0;		/* Too easy - nothing to do */
+		goto done;
+	}
+	retval = validate_change(cs, &trialcs);
+	if (retval < 0)
+		goto done;
+
+	mutex_lock(&callback_mutex);
+	cs->mems_allowed = trialcs.mems_allowed;
+	cs->mems_generation = cpuset_mems_generation++;
+	mutex_unlock(&callback_mutex);
+
+	retval = update_tasks_nodemask(cs, &oldmem);
+done:
+	return retval;
+}
+
 int current_cpuset_is_being_rebound(void)
 {
 	return task_cs(current) == cpuset_being_rebound;
@@ -1942,7 +1980,6 @@ 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.
-- 
1.5.4.rc3

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Andrew Morton]

On Wed, 04 Jun 2008 10:00:34 +0800 Miao Xie <miaox@cn.fujitsu.com> wrote:

> on 2008-6-4 8:55 Li Zefan wrote:
> > Andrew Morton wrote:
> >> On Mon, 02 Jun 2008 16:31:05 +0800
> >> Miao Xie <miaox@cn.fujitsu.com> wrote:
> >>
> >>> extract two functions from update_cpumask() and update_nodemask().They will be
> >>> used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
> >>> offline/online.
> >> Unfortunately this patch conflicts with
> >> http://userweb.kernel.org/~akpm/mmotm/broken-out/cpusets-fix-bug-when-adding-nonexistent-cpu-or-mem.patch
> >>
> >> Please check that the patch which I merged still makes sense.  ie: that
> >> we did not revert the effects of that bugfix.
> >>
> > 
> > They are fixing 2 different bugs, so they are irrelated, but unfortunately conflict in
> > the code due to Miao's first patch which does code restructuring.
> > 
> 
> Right. I will modify my patches.

I already did that, didn't I?

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Li Zefan]

Andrew Morton wrote:
> On Wed, 04 Jun 2008 10:00:34 +0800 Miao Xie <miaox@cn.fujitsu.com> wrote:
> 
>> on 2008-6-4 8:55 Li Zefan wrote:
>>> Andrew Morton wrote:
>>>> On Mon, 02 Jun 2008 16:31:05 +0800
>>>> Miao Xie <miaox@cn.fujitsu.com> wrote:
>>>>
>>>>> extract two functions from update_cpumask() and update_nodemask().They will be
>>>>> used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
>>>>> offline/online.
>>>> Unfortunately this patch conflicts with
>>>> http://userweb.kernel.org/~akpm/mmotm/broken-out/cpusets-fix-bug-when-adding-nonexistent-cpu-or-mem.patch
>>>>
>>>> Please check that the patch which I merged still makes sense.  ie: that
>>>> we did not revert the effects of that bugfix.
>>>>
>>> They are fixing 2 different bugs, so they are irrelated, but unfortunately conflict in
>>> the code due to Miao's first patch which does code restructuring.
>>>
>> Right. I will modify my patches.
> 
> I already did that, didn't I?
> 

Oh, I just had a look at that, and yes you did that, but in a wrong way. :(

You reverted the effects of this bugfix:
	cpusets-fix-bug-when-adding-nonexistent-cpu-or-mem.patch

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Paul Jackson]

Andrew wrote:
> I already did that, didn't I?

Where can I find your attempted merge, Andrew?

Probably someplace I should know about ... sorry.

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

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Andrew Morton]

On Tue, 3 Jun 2008 21:59:17 -0500 Paul Jackson <pj@sgi.com> wrote:

> Andrew wrote:
> > I already did that, didn't I?
> 
> Where can I find your attempted merge, Andrew?
> 
> Probably someplace I should know about ... sorry.
> 

Who are you and what do you have to do with cpusets?

Sigh, sorry.  I need to drop and redo them anyway.

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Andrew Morton]

On Tue, 3 Jun 2008 21:00:38 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 3 Jun 2008 21:59:17 -0500 Paul Jackson <pj@sgi.com> wrote:
> 
> > Andrew wrote:
> > > I already did that, didn't I?
> > 
> > Where can I find your attempted merge, Andrew?
> > 
> > Probably someplace I should know about ... sorry.
> > 
> 
> Who are you and what do you have to do with cpusets?
> 
> Sigh, sorry.  I need to drop and redo them anyway.

hey, who are you calling senile?  You were cc'ed.

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Paul Jackson]

Andrew, replying to Andrew:
> > Sigh, sorry.  I need to drop and redo them anyway.
> 
> hey, who are you calling senile?  You were cc'ed.

 ... once again demonstrating that Aussies a sense of humor
beyond the grasp of the rest of us mere mortals.

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

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Andrew Morton]

On Mon, 02 Jun 2008 16:31:05 +0800 Miao Xie <miaox@cn.fujitsu.com> wrote:

> extract two functions from update_cpumask() and update_nodemask().They will be
> used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
> offline/online.
> 
> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
> Acked-by: Paul Jackson <pj@sgi.com>

This patch has problems.

kernel/cpuset.c: In function 'cpuset_write_resmask':
kernel/cpuset.c:1374: warning: passing argument 2 of 'update_nodemask' discards qualifiers from pointer target type

Did you not get this warning also?

I don't know how to fix it.  cftype.write_string() requires a const
char* in the third arg, but we then go on to call update_nodemask(),
which does a strstrip() on this allegedly-const char array.

Taking a copy of the string in update_nodemask() would fix things, but
that's pretty lame.

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Li Zefan]

Andrew Morton wrote:
> On Mon, 02 Jun 2008 16:31:05 +0800 Miao Xie <miaox@cn.fujitsu.com> wrote:
> 
>> extract two functions from update_cpumask() and update_nodemask().They will be
>> used later for updating tasks' cpus_allowed and mems_allowed after CPU/NODE
>> offline/online.
>>
>> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
>> Acked-by: Paul Jackson <pj@sgi.com>
> 
> This patch has problems.
> 
> kernel/cpuset.c: In function 'cpuset_write_resmask':
> kernel/cpuset.c:1374: warning: passing argument 2 of 'update_nodemask' discards qualifiers from pointer target type
> 
> Did you not get this warning also?
> 
> I don't know how to fix it.  cftype.write_string() requires a const
> char* in the third arg, but we then go on to call update_nodemask(),
> which does a strstrip() on this allegedly-const char array.
> 

That strstrip() should be removed. because cgroup_write_string() already strstrip()ed
the buffer. Paul M's original patch removed that 2 strstrip()s in cpuset.c.

> Taking a copy of the string in update_nodemask() would fix things, but
> that's pretty lame.
> 

Re: [PATCH 1/2] cpusets: restructure the function update_cpumask() and update_nodemask()

[Li Zefan]

>> kernel/cpuset.c: In function 'cpuset_write_resmask':
>> kernel/cpuset.c:1374: warning: passing argument 2 of 'update_nodemask' discards qualifiers from pointer target type
>>
>> Did you not get this warning also?
>>
>> I don't know how to fix it.  cftype.write_string() requires a const
>> char* in the third arg, but we then go on to call update_nodemask(),
>> which does a strstrip() on this allegedly-const char array.
>>
> 
> That strstrip() should be removed. because cgroup_write_string() already strstrip()ed
> the buffer. Paul M's original patch removed that 2 strstrip()s in cpuset.c.
> 

Here is the patch:

--- linux-2.6.26-rc7/kernel/cpuset.c.bak	2008-07-02 17:53:07.000000000 +0800
+++ linux-2.6.26-rc7/kernel/cpuset.c	2008-07-02 17:53:23.000000000 +0800
@@ -1011,7 +1011,7 @@ 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, char *buf)
+static int update_nodemask(struct cpuset *cs, const char *buf)
 {
 	struct cpuset trialcs;
 	nodemask_t oldmem;
@@ -1032,7 +1032,6 @@ static int update_nodemask(struct cpuset
 	 * that parsing.  The validate_change() call ensures that cpusets
 	 * with tasks have memory.
 	 */
-	buf = strstrip(buf);
 	if (!*buf) {
 		nodes_clear(trialcs.mems_allowed);
 	} else {


>> Taking a copy of the string in update_nodemask() would fix things, but
>> that's pretty lame.
>>