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

Previous thread: [PATCH 2/2] cpusets: update tasks' cpus_allowed and mems_allowed after CPU/NODE offline/online by Miao Xie on Monday, June 2, 2008 - 4:33 am. (1 message)

Next thread: [RFC: 2.6 patch] always enable FW_LOADER unless EMBEDDED=y by Adrian Bunk on Monday, June 2, 2008 - 4:41 am. (1 message)
To: Andrew Morton <akpm@...>, Linux-Kernel <linux-kernel@...>
Cc: Paul Jackson <pj@...>, Paul Menage <menage@...>
Date: Monday, June 2, 2008 - 4:31 am

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_allowe...

To: <miaox@...>
Cc: Linux-Kernel <linux-kernel@...>, Paul Jackson <pj@...>, Paul Menage <menage@...>
Date: Wednesday, July 2, 2008 - 5:19 am

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.

--

To: Andrew Morton <akpm@...>
Cc: <miaox@...>, Linux-Kernel <linux-kernel@...>, Paul Jackson <pj@...>, Paul Menage <menage@...>
Date: Wednesday, July 2, 2008 - 5:41 am

That strstrip() should be removed. because cgroup_write_string() already strstrip()ed
--

To: Andrew Morton <akpm@...>
Cc: <miaox@...>, Linux-Kernel <linux-kernel@...>, Paul Jackson <pj@...>, Paul Menage <menage@...>
Date: Wednesday, July 2, 2008 - 5:54 am

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);
--

To: <miaox@...>
Cc: <linux-kernel@...>, <pj@...>, <menage@...>
Date: Tuesday, June 3, 2008 - 6:03 pm

On Mon, 02 Jun 2008 16:31:05 +0800

Unfortunately this patch conflicts with
http://userweb.kernel.org/~akpm/mmotm/broken-out/cpusets-fix-bug-when-ad...

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

Thanks.

--

To: Andrew Morton <akpm@...>
Cc: <miaox@...>, <linux-kernel@...>, <pj@...>, <menage@...>
Date: Tuesday, June 3, 2008 - 8:55 pm

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.
--

To: Li Zefan <lizf@...>
Cc: Andrew Morton <akpm@...>, <linux-kernel@...>, <pj@...>, <menage@...>
Date: Tuesday, June 3, 2008 - 10:00 pm

Right. I will modify my patches.

--

To: <miaox@...>
Cc: Li Zefan <lizf@...>, <linux-kernel@...>, <pj@...>, <menage@...>
Date: Tuesday, June 3, 2008 - 10:11 pm

I already did that, didn't I?
--

To: Andrew Morton <akpm@...>
Cc: <miaox@...>, <lizf@...>, <linux-kernel@...>, <menage@...>
Date: Tuesday, June 3, 2008 - 10:59 pm

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
--

To: Paul Jackson <pj@...>
Cc: <miaox@...>, <lizf@...>, <linux-kernel@...>, <menage@...>
Date: Wednesday, June 4, 2008 - 12:00 am

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

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

To: Paul Jackson <pj@...>, <miaox@...>, <lizf@...>, <linux-kernel@...>, <menage@...>
Date: Wednesday, June 4, 2008 - 12:02 am

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

To: Andrew Morton <akpm@...>
Cc: <miaox@...>, <lizf@...>, <linux-kernel@...>, <menage@...>
Date: Wednesday, June 4, 2008 - 12:07 am

... 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
--

To: Andrew Morton <akpm@...>
Cc: <miaox@...>, <linux-kernel@...>, <pj@...>, <menage@...>
Date: Tuesday, June 3, 2008 - 10:16 pm

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

--

To: <miaox@...>
Cc: <akpm@...>, <linux-kernel@...>, <menage@...>
Date: Monday, June 2, 2008 - 5:55 am

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
--

Previous thread: [PATCH 2/2] cpusets: update tasks' cpus_allowed and mems_allowed after CPU/NODE offline/online by Miao Xie on Monday, June 2, 2008 - 4:33 am. (1 message)

Next thread: [RFC: 2.6 patch] always enable FW_LOADER unless EMBEDDED=y by Adrian Bunk on Monday, June 2, 2008 - 4:41 am. (1 message)