linux-kernel - Re: [RFC][PATCH 2/4] sched: Have sched_class

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: <92b43ea2-6d21-fe5e-239a-c45d3484ada4@rasmusvillemoes.dk>
Date:   Fri, 20 Dec 2019 09:52:31 +0100
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     Steven Rostedt <rostedt@...dmis.org>, linux-kernel@...r.kernel.org
Cc:     Kirill Tkhai <tkhai@...dex.ru>,
        Kirill Tkhai <ktkhai@...tuozzo.com>,
        Peter Zijlstra <peterz@...radead.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "juri.lelli@...hat.com" <juri.lelli@...hat.com>,
        "vincent.guittot@...aro.org" <vincent.guittot@...aro.org>,
        "dietmar.eggemann@....com" <dietmar.eggemann@....com>,
        "bsegall@...gle.com" <bsegall@...gle.com>,
        "mgorman@...e.de" <mgorman@...e.de>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [RFC][PATCH 2/4] sched: Have sched_class_highest define by
 vmlinux.lds.h

On 19/12/2019 22.44, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@...dmis.org>
> 
> Now that the sched_class descriptors are defined by the linker script, and
> this needs to be aware of the existance of stop_sched_class when SMP is
> enabled or not, as it is used as the "highest" priority when defined. Move
> the declaration of sched_class_highest to the same location in the linker
> script that inserts stop_sched_class, and this will also make it easier to
> see what should be defined as the highest class, as this linker script
> location defines the priorities as well.
> 
> Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
> ---
>  include/asm-generic/vmlinux.lds.h | 11 ++++++++++-
>  kernel/sched/sched.h              |  9 +++------
>  2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 772d961c69a5..1c14c4ddf785 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -109,9 +109,16 @@
>  #endif
>  
>  #ifdef CONFIG_SMP
> -#define STOP_SCHED_CLASS	*(__stop_sched_class)
> +#define STOP_SCHED_CLASS		\
> +	STRUCT_ALIGN();			\
> +	sched_class_highest = .;	\
> +	*(__stop_sched_class)
> +#define BEFORE_DL_SCHED_CLASS
>  #else
>  #define STOP_SCHED_CLASS
> +#define BEFORE_DL_SCHED_CLASS		\
> +	STRUCT_ALIGN();			\
> +	sched_class_highest = .;
>  #endif
>  
>  /*
> @@ -120,9 +127,11 @@
>   * relation to each other.
>   */
>  #define SCHED_DATA				\
> +	STRUCT_ALIGN();				\
>  	*(__idle_sched_class)			\
>  	*(__fair_sched_class)			\
>  	*(__rt_sched_class)			\
> +	BEFORE_DL_SCHED_CLASS			\
>  	*(__dl_sched_class)			\
>  	STOP_SCHED_CLASS

If you reverse the ordering so the highest sched class comes first, this
can just be

sched_class_highest = .
STOP_SCHED_CLASS
*(__dl_sched_class)
...
*(__idle_sched_class)
__end_sched_classes = .

and this from patch 3/4

 #define for_class_range(class, _from, _to) \
-	for (class = (_from); class != (_to); class = class->next)
+	for (class = (_from); class > (_to); class--)

 #define for_each_class(class) \
-	for_class_range(class, &sched_class_highest, NULL)
+	for_class_range(class, &sched_class_highest, (&__start_sched_classes) - 1)

can instead become

+ for (class = (_from); class < (_to); class++)

and

+ for_class_range(class, &sched_class_highest, &__end_sched_classes)

which seem somewhat more readable.

And actually, I don't think you need the STOP_SCHED_CLASS define at all
- in non-SMP, no object file has a __stop_sched_class section, so
including *(__stop_sched_class) unconditionally will DTRT. (BTW, it's a
bit confusing that stop_task.o is compiled in if CONFIG_SMP, but
stop_task.c also has a #ifdef CONFIG_SMP).

Rasmus