Re: [RFC][PATCH 2/4] sched: Have sched_class_highest define by vmlinux.lds.h

[Rasmus Villemoes]

On 19/12/2019 22.44, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@xxxxxxxxxxx>
> 
> 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@xxxxxxxxxxx>
> ---
>  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