Messages in this thread | | | Date | Sun, 4 Jan 2009 23:46:59 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH 4/6] cpuset: don't allocate trial cpuset on stack |
| |
On Wed, 31 Dec 2008 16:36:41 +0800 Li Zefan <lizf@cn.fujitsu.com> wrote:
> Impact: cleanups, reduce stack usage > > This patch prepares for the next patch. When we convert cpuset.cpus_allowed > to cpumask_var_t, (trialcs = *cs) no longer works. > > Another result of this patch is reducing stack usage of trialcs. sizeof(*cs) > can be as large as 148 bytes on x86_64, so it's really not good to have it > on stack. >
err, what?
> --- a/kernel/cpuset.c > +++ b/kernel/cpuset.c > @@ -415,6 +415,24 @@ static int is_cpuset_subset(const struct cpuset *p, const struct cpuset *q) > is_mem_exclusive(p) <= is_mem_exclusive(q); > } > > +/** > + * alloc_trial_cpuset - allocate a trial cpuset > + * @cs: the cpuset that the trial cpuset duplicates > + */ > +static struct cpuset *alloc_trial_cpuset(const struct cpuset *cs) > +{ > + return kmemdup(cs, sizeof(*cs), GFP_KERNEL); > +}
We copy a cpuset by value?
> -static int update_cpumask(struct cpuset *cs, const char *buf) > +static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs, > + const char *buf) > { > struct ptr_heap heap; > - struct cpuset trialcs; > int retval; > int is_load_balanced; > > @@ -891,8 +909,6 @@ static int update_cpumask(struct cpuset *cs, const char *buf) > if (cs == &top_cpuset) > return -EACCES; > > - trialcs = *cs;
Yes, we already do.
That thing contains spinlocks and list_heads (at least), which cannot be copied in this way.
Seems that we're doing this gross thing because it just so happens that we only use the cpus_allowed and mems_allowed fields, and because several of the called functions require a cpuset*, but only needed a cpumask_t.
How perfectly beastly.
| |