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

[Lai Jiangshan <laijs@cn.fujitsu.com>]

cgroup_task_count() was called twice in every loop. IMO, it's not need.
task number maybe have been increased after kfree. But kfree is generally
quicker than kmalloc. So considering the increasing task number when kfree
is not so useful.

My patch has removed one cgroup_task_count() in every loop.

My patch has an additional line: fudge += fudge >> 3;
This line will reduce loop times remarkably when loop times is large.
(but also loop times is large just in theory)



Paul Jackson wrote:
> I agree that in theory, this kernel/cpuset.c update_tasks_nodemask()
> loop could loop forever, and that by forcing ntasks to keep increasing
> monotonically, this guarantees that it cannot loop forever.
> 
> I also agree that no known exploit of this exists, and doubt that
> any could be created.
> 
> I did find the added code logic to be a tad more difficult to read
> than I'd like.  How about the following patch, instead:
> 
> ---
>  kernel/cpuset.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> --- 2.6.25-mm1.orig/kernel/cpuset.c	2008-07-31 07:05:23.000000000 -0500
> +++ 2.6.25-mm1/kernel/cpuset.c	2008-07-31 07:13:48.000000000 -0500
> @@ -880,6 +880,7 @@ static int update_nodemask(struct cpuset
>  	struct task_struct *p;
>  	struct mm_struct **mmarray;
>  	int i, n, ntasks;
> +	int prev_ntasks = 0;
>  	int migrate;
>  	int fudge;
>  	int retval;
> @@ -939,7 +940,9 @@ static int update_nodemask(struct cpuset
>  	 */
>  	while (1) {
>  		ntasks = cgroup_task_count(cs->css.cgroup);  /* guess */
> +		ntasks = max(ntasks, prev_ntasks);	/* keep increasing */
>  		ntasks += fudge;
> +		prev_ntasks = ntasks;
>  		mmarray = kmalloc(ntasks * sizeof(*mmarray), GFP_KERNEL);
>  		if (!mmarray)
>  			goto done;
> 
>