lkml.org 
[lkml]   [2008]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC 07/13] sched: Reduce stack size requirements in kernel/sched.c
Peter Zijlstra wrote:
> On Sat, 2008-09-06 at 16:50 -0700, Mike Travis wrote:
>> plain text document attachment (stack-hogs-kernel_sched_c)
>> * Make the following changes to kernel/sched.c functions:
>>
>> - use node_to_cpumask_ptr in place of node_to_cpumask
>> - use get_cpumask_var for temporary cpumask_t variables
>> - use alloc_cpumask_ptr where available
>>
>> * Remove special code for SCHED_CPUMASK_ALLOC and use CPUMASK_ALLOC
>> from linux/cpumask.h.
>>
>> * The resultant stack savings are:
>>
>> ====== Stack (-l 100)
>>
>> 1 - initial
>> 2 - stack-hogs-kernel_sched_c
>> '.' is less than the limit(100)
>>
>> .1. .2. ..final..
>> 2216 -1536 680 -69% __build_sched_domains
>> 1592 -1592 . -100% move_task_off_dead_cpu
>> 1096 -1096 . -100% sched_balance_self
>> 1032 -1032 . -100% sched_setaffinity
>> 616 -616 . -100% rebalance_domains
>> 552 -552 . -100% free_sched_groups
>> 512 -512 . -100% cpu_to_allnodes_group
>> 7616 -6936 680 -91% Totals
>>
>>
>> Applies to linux-2.6.tip/master.
>>
>> Signed-off-by: Mike Travis <travis@sgi.com>
>> ---
>> kernel/sched.c | 151 ++++++++++++++++++++++++++++++---------------------------
>> 1 file changed, 81 insertions(+), 70 deletions(-)
>>
>> --- linux-2.6.tip.orig/kernel/sched.c
>> +++ linux-2.6.tip/kernel/sched.c
>> @@ -70,6 +70,7 @@
>> #include <linux/bootmem.h>
>> #include <linux/debugfs.h>
>> #include <linux/ctype.h>
>> +#include <linux/cpumask_ptr.h>
>> #include <linux/ftrace.h>
>> #include <trace/sched.h>
>>
>> @@ -117,6 +118,12 @@
>> */
>> #define RUNTIME_INF ((u64)~0ULL)
>>
>> +/*
>> + * temp cpumask variables
>> + */
>> +static DEFINE_PER_CPUMASK(temp_cpumask_1);
>> +static DEFINE_PER_CPUMASK(temp_cpumask_2);
>
> Yuck, that relies on turning preemption off everywhere you want to use
> those.
>
>
>> @@ -5384,11 +5400,14 @@ out_unlock:
>>
>> long sched_setaffinity(pid_t pid, const cpumask_t *in_mask)
>> {
>> - cpumask_t cpus_allowed;
>> - cpumask_t new_mask = *in_mask;
>> + cpumask_ptr cpus_allowed;
>> + cpumask_ptr new_mask;
>> struct task_struct *p;
>> int retval;
>>
>> + get_cpumask_var(cpus_allowed, temp_cpumask_1);
>> + get_cpumask_var(new_mask, temp_cpumask_2);
>> + *new_mask = *in_mask;
>> get_online_cpus();
>> read_lock(&tasklist_lock);
>
> BUG!
>
> get_online_cpus() can sleep, but you just disabled preemption with those
> get_cpumask_var() horribles!
>
> Couldn't be arsed to look through the rest, but I really hate this
> cpumask_ptr() stuff that relies on disabling preemption.
>
> NAK

Yeah, I really agree as well. But I wanted to start playing with using
cpumask_t pointers in some fairly straight forward manner. Linus's and
Ingo's suggestion to just bite the bullet and redefine the cpumask_t
would force a lot of changes to be made, but perhaps that's really the
way to go.

As to obtaining temp cpumask_t's (both early and late), perhaps a pool of
them would be better? I believe it could be done similar to alloc_bootmem
(but much simpler), and I don't think there's enough nesting to require a
very large pool. (4 was the largest depth I could find in io_apic.c.) Of
course, with preemption enabled then other problems arise...

One other really big use was for the "allbutself" cpumask in the send_IPI
functions. I think here, preemption is ok because the ownership of the
cpumask temp is very short lived.

But thanks for pointing out the get_online_cpus problem. I did try and
chase down as many call trees as I could, but I obviously missed one
important one.

And thanks for looking it over!
Mike



\
 
 \ /
  Last update: 2008-09-08 16:57    [W:0.144 / U:0.732 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site