Re: [PATCH 3/6] cpuset: convert cpuset_attach() to usecpumask_var_t

[Andrew Morton]

On Wed, 31 Dec 2008 16:36:11 +0800 Li Zefan <lizf@xxxxxxxxxxxxxx> wrote:

> Impact: reduce stack usage
> 
> Allocate a cpumask_var_t in cpuset_can_attach() and then use it in
> cpuset_attach(), so we won't fail cpuset_attach().
> 
> Signed-off-by: Li Zefan <lizf@xxxxxxxxxxxxxx>
> ---
>  kernel/cpuset.c |   31 +++++++++++++++++++++++++------
>  1 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/cpuset.c b/kernel/cpuset.c
> index afa29cf..b4c36d5 100644
> --- a/kernel/cpuset.c
> +++ b/kernel/cpuset.c
> @@ -1306,6 +1306,14 @@ static int fmeter_getrate(struct fmeter *fmp)
>  	return val;
>  }
>  
> +/*
> + * Since cpuset_attach() can't fail, we allocate a cpumask_var_t in
> + * cpuset_can_attach() and then use it in cpuset_attach().
> + *
> + * Protected by cgroup_lock.

OK, but it's a bit fragile.

How are we to ensure that all calls to cpuset_can_attach() are followed
by a call to cpuset_attach(), and how are we to ensure that both calls
are protected by the same taking of the lock?  For all time?

In fact, it's broken right now, isn't it?  If cgroup_attach_task()'s
call to find_css_set() fails, we leak the result of cpuset_can_attach()'s
alloc_cpumask_var()?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/