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: <20250109085539.C8tx_cYr@linutronix.de>
Date: Thu, 9 Jan 2025 09:55:39 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: linux-kernel@...r.kernel.org, linux-rt-devel@...ts.linux.dev
Cc: Ben Segall <bsegall@...gle.com>,
	Catalin Marinas <catalin.marinas@....com>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Ingo Molnar <mingo@...hat.com>, Juri Lelli <juri.lelli@...hat.com>,
	Mel Gorman <mgorman@...e.de>, Peter Zijlstra <peterz@...radead.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Valentin Schneider <vschneid@...hat.com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Will Deacon <will@...nel.org>, x86@...nel.org
Subject: Re: [RFC PATCH] preempt: Add a generic function to return the
 preemption string.

On 2024-12-06 12:34:31 [+0100], To linux-kernel@...r.kernel.org 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.
> This is an attempt to cover all users that I identified so far with a
> generic function provided by the scheduler. While at it, extend it with
> the LAZY information and add it to dump_stack_print_info().
> 
> Comments?

any comments on the approach or should I just repost it without the RFC?

> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> ---
>  arch/arm/kernel/traps.c      | 11 ++---------
>  arch/arm64/kernel/traps.c    | 12 ++----------
>  arch/powerpc/kernel/traps.c  |  4 ++--
>  arch/s390/kernel/dumpstack.c |  9 ++-------
>  arch/x86/kernel/dumpstack.c  |  7 +------
>  arch/xtensa/kernel/traps.c   |  6 +-----
>  include/linux/preempt.h      |  2 ++
>  kernel/sched/core.c          | 24 ++++++++++++++++++++++++
>  kernel/trace/trace.c         |  6 +-----
>  lib/dump_stack.c             |  4 ++--
>  10 files changed, 39 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index 6ea645939573f..1254992184d2e 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -258,13 +258,6 @@ void show_stack(struct task_struct *tsk, unsigned long *sp, const char *loglvl)
>  	barrier();
>  }
>  
> -#ifdef CONFIG_PREEMPT
> -#define S_PREEMPT " PREEMPT"
> -#elif defined(CONFIG_PREEMPT_RT)
> -#define S_PREEMPT " PREEMPT_RT"
> -#else
> -#define S_PREEMPT ""
> -#endif
>  #ifdef CONFIG_SMP
>  #define S_SMP " SMP"
>  #else
> @@ -282,8 +275,8 @@ static int __die(const char *str, int err, struct pt_regs *regs)
>  	static int die_counter;
>  	int ret;
>  
> -	pr_emerg("Internal error: %s: %x [#%d]" S_PREEMPT S_SMP S_ISA "\n",
> -	         str, err, ++die_counter);
> +	pr_emerg("Internal error: %s: %x [#%d] %s" S_SMP S_ISA "\n",
> +		 str, err, ++die_counter, preempt_model_str());
>  
>  	/* trap and error numbers are mostly meaningless on ARM */
>  	ret = notify_die(DIE_OOPS, str, regs, err, tsk->thread.trap_no, SIGSEGV);
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 4e26bd356a482..0b6f92fcdb304 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -172,14 +172,6 @@ static void dump_kernel_instr(const char *lvl, struct pt_regs *regs)
>  	printk("%sCode: %s\n", lvl, str);
>  }
>  
> -#ifdef CONFIG_PREEMPT
> -#define S_PREEMPT " PREEMPT"
> -#elif defined(CONFIG_PREEMPT_RT)
> -#define S_PREEMPT " PREEMPT_RT"
> -#else
> -#define S_PREEMPT ""
> -#endif
> -
>  #define S_SMP " SMP"
>  
>  static int __die(const char *str, long err, struct pt_regs *regs)
> @@ -187,8 +179,8 @@ static int __die(const char *str, long err, struct pt_regs *regs)
>  	static int die_counter;
>  	int ret;
>  
> -	pr_emerg("Internal error: %s: %016lx [#%d]" S_PREEMPT S_SMP "\n",
> -		 str, err, ++die_counter);
> +	pr_emerg("Internal error: %s: %016lx [#%d] %s" S_SMP "\n",
> +		 str, err, ++die_counter, preempt_model_str());
>  
>  	/* trap and error numbers are mostly meaningless on ARM */
>  	ret = notify_die(DIE_OOPS, str, regs, err, 0, SIGSEGV);
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index edf5cabe5dfdb..d6d77d92b3358 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -263,10 +263,10 @@ static int __die(const char *str, struct pt_regs *regs, long err)
>  {
>  	printk("Oops: %s, sig: %ld [#%d]\n", str, err, ++die_counter);
>  
> -	printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s %s\n",
> +	printk("%s PAGE_SIZE=%luK%s %s %s%s%s%s %s\n",
>  	       IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) ? "LE" : "BE",
>  	       PAGE_SIZE / 1024, get_mmu_str(),
> -	       IS_ENABLED(CONFIG_PREEMPT) ? " PREEMPT" : "",
> +	       preempt_model_str(),
>  	       IS_ENABLED(CONFIG_SMP) ? " SMP" : "",
>  	       IS_ENABLED(CONFIG_SMP) ? (" NR_CPUS=" __stringify(NR_CPUS)) : "",
>  	       debug_pagealloc_enabled() ? " DEBUG_PAGEALLOC" : "",
> diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
> index 1ecd0580561f6..7930fbab69dbb 100644
> --- a/arch/s390/kernel/dumpstack.c
> +++ b/arch/s390/kernel/dumpstack.c
> @@ -198,13 +198,8 @@ void __noreturn die(struct pt_regs *regs, const char *str)
>  	console_verbose();
>  	spin_lock_irq(&die_lock);
>  	bust_spinlocks(1);
> -	printk("%s: %04x ilc:%d [#%d] ", str, regs->int_code & 0xffff,
> -	       regs->int_code >> 17, ++die_counter);
> -#ifdef CONFIG_PREEMPT
> -	pr_cont("PREEMPT ");
> -#elif defined(CONFIG_PREEMPT_RT)
> -	pr_cont("PREEMPT_RT ");
> -#endif
> +	printk("%s: %04x ilc:%d [#%d] %s", str, regs->int_code & 0xffff,
> +	       regs->int_code >> 17, ++die_counter, preempt_model_str());
>  	pr_cont("SMP ");
>  	if (debug_pagealloc_enabled())
>  		pr_cont("DEBUG_PAGEALLOC");
> diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
> index a7d562697e50e..064b23a93c6fe 100644
> --- a/arch/x86/kernel/dumpstack.c
> +++ b/arch/x86/kernel/dumpstack.c
> @@ -395,18 +395,13 @@ NOKPROBE_SYMBOL(oops_end);
>  
>  static void __die_header(const char *str, struct pt_regs *regs, long err)
>  {
> -	const char *pr = "";
> -
>  	/* Save the regs of the first oops for the executive summary later. */
>  	if (!die_counter)
>  		exec_summary_regs = *regs;
>  
> -	if (IS_ENABLED(CONFIG_PREEMPTION))
> -		pr = IS_ENABLED(CONFIG_PREEMPT_RT) ? " PREEMPT_RT" : " PREEMPT";
> -
>  	printk(KERN_DEFAULT
>  	       "Oops: %s: %04lx [#%d]%s%s%s%s%s\n", str, err & 0xffff,
> -	       ++die_counter, pr,
> +	       ++die_counter, preempt_model_str(),
>  	       IS_ENABLED(CONFIG_SMP)     ? " SMP"             : "",
>  	       debug_pagealloc_enabled()  ? " DEBUG_PAGEALLOC" : "",
>  	       IS_ENABLED(CONFIG_KASAN)   ? " KASAN"           : "",
> diff --git a/arch/xtensa/kernel/traps.c b/arch/xtensa/kernel/traps.c
> index 38092d21acf8e..0edba7d8df8c7 100644
> --- a/arch/xtensa/kernel/traps.c
> +++ b/arch/xtensa/kernel/traps.c
> @@ -629,15 +629,11 @@ DEFINE_SPINLOCK(die_lock);
>  void __noreturn die(const char * str, struct pt_regs * regs, long err)
>  {
>  	static int die_counter;
> -	const char *pr = "";
> -
> -	if (IS_ENABLED(CONFIG_PREEMPTION))
> -		pr = IS_ENABLED(CONFIG_PREEMPT_RT) ? " PREEMPT_RT" : " PREEMPT";
>  
>  	console_verbose();
>  	spin_lock_irq(&die_lock);
>  
> -	pr_info("%s: sig: %ld [#%d]%s\n", str, err, ++die_counter, pr);
> +	pr_info("%s: sig: %ld [#%d]%s\n", str, err, ++die_counter, preempt_model_str());
>  	show_regs(regs);
>  	if (!user_mode(regs))
>  		show_stack(NULL, (unsigned long *)regs->areg[1], KERN_INFO);
> 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 95e40895a5190..8f5517dbe07d4 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -7642,6 +7642,30 @@ static inline void preempt_dynamic_init(void) { }
>  
>  #endif /* CONFIG_PREEMPT_DYNAMIC */
>  
> +const char *preempt_model_str(void)
> +{
> +	if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) && preempt_model_lazy()) {
> +		if (preempt_model_rt())
> +			return "PREEMPT_RT+LAZY";
> +		if (preempt_model_full())
> +			return "PREEMPT+LAZY";
> +		if (preempt_model_voluntary())
> +			return "VOLUNTARY+LAZY";
> +		if (preempt_model_none())
> +			return "NONE+LAZY";
> +	} else {
> +		if (preempt_model_rt())
> +			return "PREEMPT_RT";
> +		if (preempt_model_full())
> +			return "PREEMPT";
> +		if (preempt_model_voluntary())
> +			return "VOLUNTARY";
> +		if (preempt_model_none())
> +			return "NONE";
> +	}
> +	return "UNKNOWN-PREEMPT";
> +}
> +
>  int io_schedule_prepare(void)
>  {
>  	int old_iowait = current->in_iowait;
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index be62f0ea1814d..3861f53f9a434 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -4266,11 +4266,7 @@ print_trace_header(struct seq_file *m, struct trace_iterator *iter)
>  		   entries,
>  		   total,
>  		   buf->cpu,
> -		   preempt_model_none()      ? "server" :
> -		   preempt_model_voluntary() ? "desktop" :
> -		   preempt_model_full()      ? "preempt" :
> -		   preempt_model_rt()        ? "preempt_rt" :
> -		   "unknown",
> +		   preempt_model_str(),
>  		   /* These are reserved for later use */
>  		   0, 0, 0, 0);
>  #ifdef CONFIG_SMP
> diff --git a/lib/dump_stack.c b/lib/dump_stack.c
> index 388da1aea14a5..c3e59f8992279 100644
> --- a/lib/dump_stack.c
> +++ b/lib/dump_stack.c
> @@ -54,7 +54,7 @@ void __init dump_stack_set_arch_desc(const char *fmt, ...)
>   */
>  void dump_stack_print_info(const char *log_lvl)
>  {
> -	printk("%sCPU: %d UID: %u PID: %d Comm: %.20s %s%s %s %.*s" BUILD_ID_FMT "\n",
> +	printk("%sCPU: %d UID: %u PID: %d Comm: %.20s %s%s %s %.*s %s" BUILD_ID_FMT "\n",
>  	       log_lvl, raw_smp_processor_id(),
>  	       __kuid_val(current_real_cred()->euid),
>  	       current->pid, current->comm,
> @@ -62,7 +62,7 @@ void dump_stack_print_info(const char *log_lvl)
>  	       print_tainted(),
>  	       init_utsname()->release,
>  	       (int)strcspn(init_utsname()->version, " "),
> -	       init_utsname()->version, BUILD_ID_VAL);
> +	       init_utsname()->version, preempt_model_str(), BUILD_ID_VAL);
>  
>  	if (get_taint())
>  		printk("%s%s\n", log_lvl, print_tainted_verbose());
> -- 
> 2.45.2
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ