cpusets: restructure the function update_cpumask() and update_nodemask()
PATCH 1/2 cpusets: restructure the function update_cpumask() and update_nodemask()
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.
--- kernel/cpuset.c | 179 +++++++++++++++++++++++++++++++++---------------------- 1 files changed, 108 insertions(+), 71 deletions(-)
kernel/cpuset.c +108 -71
@@ -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.
Signature · 1 line
1.5.4.rc3
Good work, Miao.
Andrew - these two patches of Miao should be ready for you to consider now.
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.940.382.4214Quoted 3 lines · On Mon, 02 Jun 2008 16:31:05 +0800 Miao Xie <[email protected]> wrote:
extract two functions from
update_cpumask()and update_nodemask().They will be used later for updating tasks'cpus_allowedandmems_allowedafter 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.
Quoted 7 lines · Andrew Morton
Quoted 3 lines · On Mon, 02 Jun 2008 16:31:05 +0800 Miao Xie <[email protected]> wrote:
extract two functions from
update_cpumask()and update_nodemask().They will be used later for updating tasks'cpus_allowedandmems_allowedafter CPU/NODE offline/online.
Quoted 6 lines
Unfortunately this patch conflicts with
http://userweb.kernel.org/~akpm/mmotm/broken-out/cpusets-fix-bug-when-adding-nonexistent-cpu-or-mem.patchPlease 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.
Quoted 10 lines · on 2008-6-4 8:55 Li Zefan wrote:
Quoted 9 lines · Andrew Morton
Quoted 3 lines · On Mon, 02 Jun 2008 16:31:05 +0800 Miao Xie <[email protected]> wrote:
extract two functions from
update_cpumask()and update_nodemask().They will be used later for updating tasks'cpus_allowedandmems_allowedafter 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
Quoted 7 lines
Quoted 3 lines
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.
Quoted 15 lines · On Wed, 04 Jun 2008 10:00:34 +0800 Miao Xie <[email protected]> wrote:
Quoted 14 lines · on 2008-6-4 8:55 Li Zefan wrote:
Quoted 12 lines · Andrew Morton
Quoted 3 lines · On Mon, 02 Jun 2008 16:31:05 +0800 Miao Xie <[email protected]> wrote:
extract two functions from
update_cpumask()and update_nodemask().They will be used later for updating tasks'cpus_allowedandmems_allowedafter 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.patchPlease check that the patch which I merged still makes sense. ie: that we did not revert the effects of that bugfix.
Quoted 5 lines
Quoted 3 lines
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?
Quoted 16 lines · Andrew Morton
Quoted 14 lines · On Wed, 04 Jun 2008 10:00:34 +0800 Miao Xie <[email protected]> wrote:
Quoted 13 lines · on 2008-6-4 8:55 Li Zefan wrote:
Quoted 12 lines · Andrew Morton
Quoted 3 lines · On Mon, 02 Jun 2008 16:31:05 +0800 Miao Xie <[email protected]> wrote:
extract two functions from
update_cpumask()and update_nodemask().They will be used later for updating tasks'cpus_allowedandmems_allowedafter 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.patchPlease check that the patch which I merged still makes sense. ie: that we did not revert the effects of that bugfix.
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
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
Quoted 1 line · Andrew
I already did that, didn't I?
Where can I find your attempted merge, Andrew?
Probably someplace I should know about ... sorry.
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.940.382.4214Quoted 7 lines · On Tue, 3 Jun 2008 21:59:17 -0500 Paul Jackson <[email protected]> wrote:
Quoted 1 line · Andrew
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.
Quoted 5 lines · On Tue, 3 Jun 2008 21:00:38 -0700 Andrew Morton <[email protected]>…
Quoted 3 lines · On Tue, 3 Jun 2008 21:59:17 -0500 Paul Jackson <[email protected]> wrote:
Quoted 1 line · Andrew
I already did that, didn't I?
Quoted 8 lines
Quoted 4 lines
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.
Andrew, replying to Andrew:
Quoted 3 lines
Quoted 1 line
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.
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.940.382.4214Quoted 6 lines · On Mon, 02 Jun 2008 16:31:05 +0800 Miao Xie <[email protected]> wrote:
extract two functions from
update_cpumask()and update_nodemask().They will be used later for updating tasks'cpus_allowedandmems_allowedafter CPU/NODE offline/online.Signed-off-by Miao Xie <[email protected]>Acked-by Paul Jackson <[email protected]>
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.
Quoted 11 lines · Andrew Morton
Quoted 6 lines · On Mon, 02 Jun 2008 16:31:05 +0800 Miao Xie <[email protected]> wrote:
extract two functions from
update_cpumask()and update_nodemask().They will be used later for updating tasks'cpus_allowedandmems_allowedafter CPU/NODE offline/online.Signed-off-by Miao Xie <[email protected]>Acked-by Paul Jackson <[email protected]>This patch has problems.
Quoted 9 lines
kernel/cpuset.c: In function 'cpuset_write_resmask':kernel/cpuset.c:1374: warning: passing argument 2 of 'update_nodemask' discards qualifiers from pointer target typeDid 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 callupdate_nodemask(), which does astrstrip()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.
Quoted 3 lines
Taking a copy of the string in
update_nodemask()would fix things, but that's pretty lame.
Quoted 5 lines
Quoted 5 lines
kernel/cpuset.c: In function 'cpuset_write_resmask':kernel/cpuset.c:1374: warning: passing argument 2 of 'update_nodemask' discards qualifiers from pointer target typeDid you not get this warning also?
Quoted 8 lines
Quoted 4 lines
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 callupdate_nodemask(), which does astrstrip()on this allegedly-const char array.That
strstrip()should be removed. becausecgroup_write_string()already strstrip()ed the buffer. Paul M's original patch removed that 2 strstrip()s incpuset.c.
Here is the patch:
kernel/cpuset.c renamed from kernel/cpuset.c.bak +1 -2
@@ -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 {
Quoted 3 lines
Quoted 3 lines
Taking a copy of the string in
update_nodemask()would fix things, but that's pretty lame.