Re: [PATCH v2 1/9] sched: Add a generic function to return the preemption string.

[Shrikanth Hegde <sshegde@linux.ibm.com>]

On 2/3/25 19:46, Sebastian Andrzej Siewior wrote:
> The individual architectures often add the preemption model to the begin
> of the backtrace. This is the case on X86 or ARM64 for the "die" case
> but not for regular warning. With the addition of DYNAMIC_PREEMPT for
> PREEMPT_RT we end up with CONFIG_PREEMPT and CONFIG_PREEMPT_RT set
> simultaneously. That means that everyone who tried to add that piece of
> information gets it wrong for PREEMPT_RT because PREEMPT is checked
> first.
> 
> Provide a generic function which returns the current scheduling model
> considering LAZY preempt and the current state of PREEMPT_DYNAMIC.
> [ The dynamic building of the string can lead to an empty string if the
>    function is invoked simultaneously on two CPUs. ]
> 
> Co-developed-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Co-developed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   include/linux/preempt.h |  2 ++
>   kernel/sched/core.c     | 47 +++++++++++++++++++++++++++++++++++++++++
>   kernel/sched/debug.c    | 10 +++++----
>   kernel/sched/sched.h    |  1 +
>   4 files changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/preempt.h b/include/linux/preempt.h
> index ca86235ac15c0..3e9808f2b5491 100644
> --- a/include/linux/preempt.h
> +++ b/include/linux/preempt.h
> @@ -515,6 +515,8 @@ static inline bool preempt_model_rt(void)
>   	return IS_ENABLED(CONFIG_PREEMPT_RT);
>   }
>   
> +extern const char *preempt_model_str(void);
> +
>   /*
>    * Does the preemption model allow non-cooperative preemption?
>    *
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 165c90ba64ea9..bd2664755b09a 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -7646,10 +7646,57 @@ PREEMPT_MODEL_ACCESSOR(lazy);
>   
>   #else /* !CONFIG_PREEMPT_DYNAMIC: */
>   
> +#define preempt_dynamic_mode -1
> +
>   static inline void preempt_dynamic_init(void) { }
>   
>   #endif /* CONFIG_PREEMPT_DYNAMIC */
>   
> +const char *preempt_modes[] = {
> +	"none", "voluntary", "full", "lazy", NULL,
> +};
> +
> +const char *preempt_model_str(void)
> +{
> +	bool brace = IS_ENABLED(CONFIG_PREEMPT_RT) &&
> +		(IS_ENABLED(CONFIG_PREEMPT_DYNAMIC) ||
> +		 IS_ENABLED(CONFIG_PREEMPT_LAZY));
> +	static char buf[128];
> +
> +	if (IS_ENABLED(CONFIG_PREEMPT_BUILD)) {
> +		struct seq_buf s;
> +
> +		seq_buf_init(&s, buf, 128);
> +		seq_buf_puts(&s, "PREEMPT");
> +
> +		if (IS_ENABLED(CONFIG_PREEMPT_RT))
> +			seq_buf_printf(&s, "%sRT%s",
> +				       brace ? "_{" : "_",
> +				       brace ? "," : "");
> +
> +		if (IS_ENABLED(CONFIG_PREEMPT_DYNAMIC)) {
> +			seq_buf_printf(&s, "(%s)%s",
> +				       preempt_dynamic_mode > 0 ?
> +				       preempt_modes[preempt_dynamic_mode] : "undef",
> +				       brace ? "}" : "");
> +			return seq_buf_str(&s);
> +		}
> +
> +		if (IS_ENABLED(CONFIG_PREEMPT_LAZY)) {
> +			seq_buf_printf(&s, "LAZY%s",
> +				       brace ? "}" : "");
> +			return seq_buf_str(&s);
> +		}
> +
> +		return seq_buf_str(&s);
> +	}
> +
> +	if (IS_ENABLED(CONFIG_PREEMPT_VOLUNTARY_BUILD))
> +		return "VOLUNTARY";
> +
> +	return "NONE";
> +}

nit: This means when it preempt=full, it is going to show PREEMPT or 
PREEMPT_RT. Maybe that could be put into changelog or add code similar 
to lazy for full as well, so it is easier for user to know if it is in 
preempt=full model. In all other models, there is a suffix like 
PREEMPTLAZY. Dynamic modes looks good.

Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>

> +
>   int io_schedule_prepare(void)
>   {
>   	int old_iowait = current->in_iowait;
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index fd7e852207157..755232ac88de9 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -244,11 +244,13 @@ static ssize_t sched_dynamic_write(struct file *filp, const char __user *ubuf,
>   
>   static int sched_dynamic_show(struct seq_file *m, void *v)
>   {
> -	static const char * preempt_modes[] = {
> -		"none", "voluntary", "full", "lazy",
> -	};
> -	int j = ARRAY_SIZE(preempt_modes) - !IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY);
>   	int i = IS_ENABLED(CONFIG_PREEMPT_RT) * 2;
> +	int j;
> +
> +	/* Count entries in NULL terminated preempt_modes */
> +	for (j = 0; preempt_modes[j]; j++)
> +		;
> +	j -= !IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY);
>   
>   	for (; i < j; i++) {
>   		if (preempt_dynamic_mode == i)
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 38e0e323dda26..fb4486ceb978a 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3619,6 +3619,7 @@ extern int preempt_dynamic_mode;
>   extern int sched_dynamic_mode(const char *str);
>   extern void sched_dynamic_update(int mode);
>   #endif
> +extern const char *preempt_modes[];
>   
>   #ifdef CONFIG_SCHED_MM_CID
>