linux-kernel - Re: [RFC 07/13] sched: Reduce stack size requirements in kernel/sched.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1220783087.8687.73.camel@twins.programming.kicks-ass.net>
Date:	Sun, 07 Sep 2008 12:24:47 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Mike Travis <travis@....com>
Cc:	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	davej@...emonkey.org.uk, David Miller <davem@...emloft.net>,
	Eric Dumazet <dada1@...mosbay.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Jack Steiner <steiner@....com>,
	Jeremy Fitzhardinge <jeremy@...p.org>,
	Jes Sorensen <jes@....com>, "H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC 07/13] sched: Reduce stack size requirements in
 kernel/sched.c

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@....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

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