Re: [PATCH] cpuset: make ntasks to be a monotonic increasing value

Lai Jiangshan
2008-08-01T01:37:32Z

Paul Jackson wrote:
> Bottom line - my priorities for non-critical code paths, most important first:
> 1) It must work.
> 2) Keep it easy for humans to understand.
> 3) Reduce kernel text size.
> 4) Reduce CPU cycles.
>

I agree, Thank you!

How about this one?

The loop after this patch applied is:

ntasks_now = cgroup_task_count(cs->css.cgroup);
while (1) {
ntasks = ntasks_now; /* guess */
ntasks += fudge;
mmarray = kmalloc(ntasks * sizeof(*mmarray), GFP_KERNEL);
if (!mmarray)
goto done;
read_lock(&tasklist_lock); /* block fork */
ntasks_now = cgroup_task_count(cs->css.cgroup);
if (ntasks_now <= ntasks)
break; /* got enough */
read_unlock(&tasklist_lock); /* try again */
kfree(mmarray);
}

I think the readability of this code is as good as original's.
And it's better in semantic meaning.

The old non monotonic increasing value is caused by the redundant
cgroup_task_count(). Removing it is good for keeping ntasks
increasing(not need additional arithmetic compare or max statement).

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/kernel/cpuset.c b/kernel/cpuset.c
index d5ab79c..56a057f 100644
--- a/kernel/cpuset.c
+++ b/kernel/cpuset.c
@@ -930,7 +930,7 @@ static int update_tasks_nodemask(struct cpuset *cs, const nodemask_t *oldmem)
{
struct task_struct *p;
struct mm_struct **mmarray;
- int i, n, ntasks;
+ int i, n, ntasks, ntasks_now;
int migrate;
int fudge;
struct cgroup_iter it;
@@ -949,14 +949,16 @@ static int update_tasks_nodemask(struct cpuset *cs, const nodemask_t *oldmem)
* few more lines of code, we can retry until we get a big
* enough mmarray[] w/o using GFP_ATOMIC.
*/
+ ntasks_now = cgroup_task_count(cs->css.cgroup);
while (1) {
- ntasks = cgroup_task_count(cs->css.cgroup); /* guess */
+ ntasks = ntasks_now; /* guess */
ntasks += fudge;
mmarray = kmalloc(ntasks * sizeof(*mmarray), GFP_KERNEL);
if (!mmarray)
goto done;
read_lock(&tasklist_lock); /* block fork */
- if (cgroup_task_count(cs->css.cgroup) <= ntasks)
+ ntasks_now = cgroup_task_count(cs->css.cgroup);
+ if (ntasks_now <= ntasks)
break; /* got enough */
read_unlock(&tasklist_lock); /* try again */
kfree(mmarray);