[PATCH v2 0/9] preempt: Add a generic function to return the preemption string.

[PATCH v2 0/9] preempt: Add a generic function to return the preemption string.

[Sebastian Andrzej Siewior]

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 series adds a generic helper which is used by
dump_stack_print_info() and other architectures which don't use the
function and provide this information on their own.

RFC…v2 https://lore.kernel.org/all/20241206113431.Q-VXMlru@linutronix.de/
  - Split into individual patches.
  - Implement preempt_model_str() based on feedback by Peter Zijlstra
    and Steven Rostedt.

Sebastian

Sebastian Andrzej Siewior (9):
  sched: Add a generic function to return the preemption string.
  lib/dump_stack: Use preempt_model_str().
  arm: Rely on generic printing of preemption model.
  arm64: Rely on generic printing of preemption model.
  powerpc: Use preempt_model_str().
  s390: Rely on generic printing of preemption model.
  x86: Rely on generic printing of preemption model.
  xtensa: Use preempt_model_str().
  tracing: Use preempt_model_str().

 arch/arm/kernel/traps.c      | 11 ++-------
 arch/arm64/kernel/traps.c    | 10 +-------
 arch/powerpc/kernel/traps.c  |  4 +--
 arch/s390/kernel/dumpstack.c |  9 ++-----
 arch/x86/kernel/dumpstack.c  |  9 ++-----
 arch/xtensa/kernel/traps.c   |  6 +----
 include/linux/preempt.h      |  2 ++
 kernel/sched/core.c          | 47 ++++++++++++++++++++++++++++++++++++
 kernel/sched/debug.c         | 10 +++++---
 kernel/sched/sched.h         |  1 +
 kernel/trace/trace.c         |  7 +-----
 lib/dump_stack.c             |  4 +--
 12 files changed, 69 insertions(+), 51 deletions(-)

-- 
2.47.2

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

[Sebastian Andrzej Siewior]

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";
+}
+
 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
 
-- 
2.47.2

[PATCH v2 2/9] lib/dump_stack: Use preempt_model_str().

[Sebastian Andrzej Siewior]

Use preempt_model_str() to print the current preemption model. Use
pr_warn() instead of printk() to pass a loglevel. This makes it part of
generic WARN/ BUG traces.

Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 lib/dump_stack.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index 388da1aea14a5..425556f3fd018 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",
+	pr_warn("%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.47.2

[PATCH v2 3/9] arm: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

__die() invokes later __show_regs() -> show_regs_print_info() which
prints the current preemption model.
Remove it from the initial line.

Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm/kernel/traps.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 6ea645939573f..afbd2ebe5c39d 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_SMP S_ISA "\n",
+		 str, err, ++die_counter);
 
 	/* trap and error numbers are mostly meaningless on ARM */
 	ret = notify_die(DIE_OOPS, str, regs, err, tsk->thread.trap_no, SIGSEGV);
-- 
2.47.2

[PATCH v2 4/9] arm64: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

__die() invokes later show_regs() -> show_regs_print_info() which prints
the current preemption model.
Remove it from the initial line.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/arm64/kernel/traps.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 4e26bd356a482..529cff825531c 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,7 +179,7 @@ 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",
+	pr_emerg("Internal error: %s: %016lx [#%d] " S_SMP "\n",
 		 str, err, ++die_counter);
 
 	/* trap and error numbers are mostly meaningless on ARM */
-- 
2.47.2

[PATCH v2 5/9] powerpc: Use preempt_model_str().

[Sebastian Andrzej Siewior]

Use preempt_model_str() instead of manually conducting the preemption
model. Use pr_emerg() instead of printk() to pass a loglevel.

Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/powerpc/kernel/traps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index edf5cabe5dfdb..9eb383189cfb2 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",
+	pr_emerg("%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" : "",
-- 
2.47.2

[PATCH v2 6/9] s390: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

die() invokes later show_regs() -> show_regs_print_info() which prints
the current preemption model.
Remove it from the initial line.

Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/s390/kernel/dumpstack.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
index 1ecd0580561f6..542098678189e 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
+	pr_emerg("%s: %04x ilc:%d [#%d]", str, regs->int_code & 0xffff,
+		 regs->int_code >> 17, ++die_counter);
 	pr_cont("SMP ");
 	if (debug_pagealloc_enabled())
 		pr_cont("DEBUG_PAGEALLOC");
-- 
2.47.2

[PATCH v2 7/9] x86: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

After __die_header(), __die_body() is always invoked. There we have
show_regs() -> show_regs_print_info() which prints the current
preemption model.
Remove it from the initial line.

Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/kernel/dumpstack.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index a7d562697e50e..91639d1e4ec2c 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,
+	       "Oops: %s: %04lx [#%d]%s%s%s%s\n", str, err & 0xffff,
+	       ++die_counter,
 	       IS_ENABLED(CONFIG_SMP)     ? " SMP"             : "",
 	       debug_pagealloc_enabled()  ? " DEBUG_PAGEALLOC" : "",
 	       IS_ENABLED(CONFIG_KASAN)   ? " KASAN"           : "",
-- 
2.47.2

[PATCH v2 8/9] xtensa: Use preempt_model_str().

[Sebastian Andrzej Siewior]

Use preempt_model_str() instead of manually conducting the preemption
model.

Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/xtensa/kernel/traps.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/xtensa/kernel/traps.c b/arch/xtensa/kernel/traps.c
index 38092d21acf8e..a7b1e4ad112e8 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);
-- 
2.47.2

[PATCH v2 9/9] tracing: Use preempt_model_str().

[Sebastian Andrzej Siewior]

Use preempt_model_str() instead of manually conducting the preemption
model.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: linux-trace-kernel@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/trace/trace.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 1496a5ac33ae1..77271faab20e9 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4100,12 +4100,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_lazy()	     ? "lazy"    :
-		   preempt_model_rt()        ? "preempt_rt" :
-		   "unknown",
+		   preempt_model_str(),
 		   /* These are reserved for later use */
 		   0, 0, 0, 0);
 #ifdef CONFIG_SMP
-- 
2.47.2

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

[Peter Zijlstra]

On Mon, Feb 03, 2025 at 03:16:23PM +0100, 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.
> 
> This series adds a generic helper which is used by
> dump_stack_print_info() and other architectures which don't use the
> function and provide this information on their own.
> 
> RFC…v2 https://lore.kernel.org/all/20241206113431.Q-VXMlru@linutronix.de/
>   - Split into individual patches.
>   - Implement preempt_model_str() based on feedback by Peter Zijlstra
>     and Steven Rostedt.
> 
> Sebastian
> 
> Sebastian Andrzej Siewior (9):
>   sched: Add a generic function to return the preemption string.
>   lib/dump_stack: Use preempt_model_str().
>   arm: Rely on generic printing of preemption model.
>   arm64: Rely on generic printing of preemption model.
>   powerpc: Use preempt_model_str().
>   s390: Rely on generic printing of preemption model.
>   x86: Rely on generic printing of preemption model.
>   xtensa: Use preempt_model_str().
>   tracing: Use preempt_model_str().

Works for me; how do we want to go about merging this?

I can take it through the sched tree I suppose, once the various arch
people have blessed it.

Re: [PATCH v2 6/9] s390: Rely on generic printing of preemption model.

[Heiko Carstens]

On Mon, Feb 03, 2025 at 03:16:29PM +0100, Sebastian Andrzej Siewior wrote:
> die() invokes later show_regs() -> show_regs_print_info() which prints
> the current preemption model.
> Remove it from the initial line.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/s390/kernel/dumpstack.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)

Acked-by: Heiko Carstens <hca@linux.ibm.com>

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

[Sebastian Andrzej Siewior]

On 2025-02-03 15:25:05 [+0100], Peter Zijlstra wrote:
> Works for me; how do we want to go about merging this?

Thank you.

> I can take it through the sched tree I suppose, once the various arch
> people have blessed it.

I guess this is the easiest part. I would ping those who did not ACK
their bits next week.

Sebastian

Re: [PATCH v2 5/9] powerpc: Use preempt_model_str().

[Christophe Leroy]

Le 03/02/2025 à 15:16, Sebastian Andrzej Siewior a écrit :
> Use preempt_model_str() instead of manually conducting the preemption
> model. Use pr_emerg() instead of printk() to pass a loglevel.

Why use pr_emerg() for that line and not all other ones ?

The purpose of using printk() is to get it at the level defined by 
CONFIG_MESSAGE_LOGLEVEL_DEFAULT and I think it is important to have the 
full Oops block at the same level.

> 
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Naveen N Rao <naveen@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   arch/powerpc/kernel/traps.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index edf5cabe5dfdb..9eb383189cfb2 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",
> +	pr_emerg("%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" : "",

Re: [PATCH v2 5/9] powerpc: Use preempt_model_str().

[Sebastian Andrzej Siewior]

On 2025-02-03 16:19:06 [+0100], Christophe Leroy wrote:
> 
> 
> Le 03/02/2025 à 15:16, Sebastian Andrzej Siewior a écrit :
> > Use preempt_model_str() instead of manually conducting the preemption
> > model. Use pr_emerg() instead of printk() to pass a loglevel.
> 
> Why use pr_emerg() for that line and not all other ones ?

checkpatch complained for the current printk() line and this looks like
an emergency coming from die().

> The purpose of using printk() is to get it at the level defined by
> CONFIG_MESSAGE_LOGLEVEL_DEFAULT and I think it is important to have the full
> Oops block at the same level.

Okay. So "printk(KERN_DEFAULT " then.

Sebastian

Re: [PATCH v2 5/9] powerpc: Use preempt_model_str().

[Christophe Leroy]

Le 03/02/2025 à 17:01, Sebastian Andrzej Siewior a écrit :
> On 2025-02-03 16:19:06 [+0100], Christophe Leroy wrote:
>>
>>
>> Le 03/02/2025 à 15:16, Sebastian Andrzej Siewior a écrit :
>>> Use preempt_model_str() instead of manually conducting the preemption
>>> model. Use pr_emerg() instead of printk() to pass a loglevel.
>>
>> Why use pr_emerg() for that line and not all other ones ?
> 
> checkpatch complained for the current printk() line and this looks like
> an emergency coming from die().

Right but checkpatch only looks at the line you modify with your patch, 
it doesn't consider the global picture.

> 
>> The purpose of using printk() is to get it at the level defined by
>> CONFIG_MESSAGE_LOGLEVEL_DEFAULT and I think it is important to have the full
>> Oops block at the same level.
> 
> Okay. So "printk(KERN_DEFAULT " then.

Up to you, I'm fine with that but you should consistently update all 
printk's in the function, not only that one, so is it really worth it ?

Christophe

Re: [PATCH v2 9/9] tracing: Use preempt_model_str().

[Steven Rostedt]

On Mon,  3 Feb 2025 15:16:32 +0100
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> Use preempt_model_str() instead of manually conducting the preemption
> model.
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: linux-trace-kernel@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  kernel/trace/trace.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)

Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve

Re: [PATCH v2 8/9] xtensa: Use preempt_model_str().

[Max Filippov]

On Mon, Feb 3, 2025 at 6:16 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> Use preempt_model_str() instead of manually conducting the preemption
> model.
>
> Cc: Chris Zankel <chris@zankel.net>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/xtensa/kernel/traps.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)

Acked-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max

[PATCH v3 5/9] powerpc: Use preempt_model_str().

[Sebastian Andrzej Siewior]

Use preempt_model_str() instead of manually conducting the preemption
model. Use pr_emerg() instead of printk() to pass a loglevel.

Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v2…v3:
  - Use printk() instead of pr_emerg() to remain consistent with the
    other invocations in terms of printing context.

 arch/powerpc/kernel/traps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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" : "",
-- 
2.47.2

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Christophe Leroy]

Le 04/02/2025 à 09:22, Sebastian Andrzej Siewior a écrit :
> Use preempt_model_str() instead of manually conducting the preemption
> model. Use pr_emerg() instead of printk() to pass a loglevel.
> 
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Naveen N Rao <naveen@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
> v2…v3:
>    - Use printk() instead of pr_emerg() to remain consistent with the
>      other invocations in terms of printing context.
> 
>   arch/powerpc/kernel/traps.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 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" : "",

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Shrikanth Hegde]

On 2/4/25 13:52, Sebastian Andrzej Siewior wrote:
> Use preempt_model_str() instead of manually conducting the preemption
> model. Use pr_emerg() instead of printk() to pass a loglevel.

even on powerpc, i see __die ends up calling show_regs_print_info().
Why print it twice?

> 
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Naveen N Rao <naveen@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> v2…v3:
>    - Use printk() instead of pr_emerg() to remain consistent with the
>      other invocations in terms of printing context.
> 
>   arch/powerpc/kernel/traps.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 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" : "",

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

[Shrikanth Hegde]

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
>   

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Christophe Leroy]

Le 08/02/2025 à 08:35, Shrikanth Hegde a écrit :
> 
> 
> On 2/4/25 13:52, Sebastian Andrzej Siewior wrote:
>> Use preempt_model_str() instead of manually conducting the preemption
>> model. Use pr_emerg() instead of printk() to pass a loglevel.
> 
> even on powerpc, i see __die ends up calling show_regs_print_info().
> Why print it twice?

I don't understand what you mean, what is printed twice ?

I can't see show_regs_print_info() printing the preemption model, am I 
missing something ?

Christophe

> 
>>
>> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Nicholas Piggin <npiggin@gmail.com>
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: Naveen N Rao <naveen@kernel.org>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> ---
>> v2…v3:
>>    - Use printk() instead of pr_emerg() to remain consistent with the
>>      other invocations in terms of printing context.
>>
>>   arch/powerpc/kernel/traps.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> 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" : "",
> 

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Shrikanth Hegde]

On 2/8/25 18:25, Christophe Leroy wrote:
> 
> 
> Le 08/02/2025 à 08:35, Shrikanth Hegde a écrit :
>>
>>
>> On 2/4/25 13:52, Sebastian Andrzej Siewior wrote:
>>> Use preempt_model_str() instead of manually conducting the preemption
>>> model. Use pr_emerg() instead of printk() to pass a loglevel.
>>
>> even on powerpc, i see __die ends up calling show_regs_print_info().
>> Why print it twice?
> 
> I don't understand what you mean, what is printed twice ?
> 
> I can't see show_regs_print_info() printing the preemption model, am I 
> missing something ?
> 

Patch 2/9 add preemption string in dump_stack_print_info.

__die -> show_regs() _> show_regs_print_info() -> 
dump_stack_print_info() -> init_utsname()->version, preempt_model_str(), 
BUILD_ID_VAL);

Wont we end up in this path?

> Christophe
> 
>>
>>>
>>> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Nicholas Piggin <npiggin@gmail.com>
>>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>>> Cc: Naveen N Rao <naveen@kernel.org>
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>> ---
>>> v2…v3:
>>>    - Use printk() instead of pr_emerg() to remain consistent with the
>>>      other invocations in terms of printing context.
>>>
>>>   arch/powerpc/kernel/traps.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> 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" : "",
>>
> 

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Christophe Leroy]

Le 08/02/2025 à 14:42, Shrikanth Hegde a écrit :
> 
> 
> On 2/8/25 18:25, Christophe Leroy wrote:
>>
>>
>> Le 08/02/2025 à 08:35, Shrikanth Hegde a écrit :
>>>
>>>
>>> On 2/4/25 13:52, Sebastian Andrzej Siewior wrote:
>>>> Use preempt_model_str() instead of manually conducting the preemption
>>>> model. Use pr_emerg() instead of printk() to pass a loglevel.
>>>
>>> even on powerpc, i see __die ends up calling show_regs_print_info().
>>> Why print it twice?
>>
>> I don't understand what you mean, what is printed twice ?
>>
>> I can't see show_regs_print_info() printing the preemption model, am I 
>> missing something ?
>>
> 
> Patch 2/9 add preemption string in dump_stack_print_info.
> 
> __die -> show_regs() _> show_regs_print_info() -> 
> dump_stack_print_info() -> init_utsname()->version, preempt_model_str(), 
> BUILD_ID_VAL);
> 
> Wont we end up in this path?

Indeed I missed that. You are right, we now get the information twice:

[  440.068216] BUG: Unable to handle kernel data access on write at 
0xc09036fc
[  440.075051] Faulting instruction address: 0xc045ddf8
[  440.080032] Oops: Kernel access of bad area, sig: 11 [#1]
[  440.085438] BE PAGE_SIZE=16K PREEMPT  CMPC885
[  440.089872] SAF3000 DIE NOTIFICATION
[  440.093391] CPU: 0 UID: 0 PID: 472 Comm: sh Not tainted 
6.13.0-s3k-dev-01384-g54680e2fbfb0 #1379 PREEMPT
[  440.102977] Hardware name: MIAE 8xx 0x500000 CMPC885
[  440.107951] NIP:  c045ddf8 LR: c045dde8 CTR: 00000000
[  440.113015] REGS: c9bf3d60 TRAP: 0300   Not tainted 
(6.13.0-s3k-dev-01384-g54680e2fbfb0)
[  440.121215] MSR:  00009032 <EE,ME,IR,DR,RI>  CR: 35009393  XER: 80003100
[  440.128198] DAR: c09036fc DSISR: 82000000
[  440.128198] GPR00: c045c59c c9bf3e20 c27e7700 0000002e c108575c 
00000001 c1085850 00009032
[  440.128198] GPR08: 00000027 0198b861 00000001 3ffff000 55009393 
100d815e 7fcf5e20 100d0000
[  440.128198] GPR16: 100d0000 00000000 113e447c 113e4480 00000000 
00000001 00000000 00000000
[  440.128198] GPR24: 113e4464 00000000 c1828000 c9bf3ef8 c1136eac 
c9bf3ef8 c2888000 c0900000
[  440.168081] NIP [c045ddf8] lkdtm_WRITE_RO+0x34/0x50
[  440.172969] LR [c045dde8] lkdtm_WRITE_RO+0x24/0x50
[  440.177771] Call Trace:

Christophe

> 
>> Christophe
>>
>>>
>>>>
>>>> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
>>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>>> Cc: Nicholas Piggin <npiggin@gmail.com>
>>>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>> Cc: Naveen N Rao <naveen@kernel.org>
>>>> Cc: linuxppc-dev@lists.ozlabs.org
>>>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>>>> ---
>>>> v2…v3:
>>>>    - Use printk() instead of pr_emerg() to remain consistent with the
>>>>      other invocations in terms of printing context.
>>>>
>>>>   arch/powerpc/kernel/traps.c | 4 ++--
>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> 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" : "",
>>>
>>
> 

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Shrikanth Hegde]

On 2/8/25 23:25, Christophe Leroy wrote:
> 
> 
> Le 08/02/2025 à 14:42, Shrikanth Hegde a écrit :
>>
>>
>> On 2/8/25 18:25, Christophe Leroy wrote:
>>>
>>>
>>> Le 08/02/2025 à 08:35, Shrikanth Hegde a écrit :
>>>>
>>>>
>>>> On 2/4/25 13:52, Sebastian Andrzej Siewior wrote:
>>>>> Use preempt_model_str() instead of manually conducting the preemption
>>>>> model. Use pr_emerg() instead of printk() to pass a loglevel.
>>>>
>>>> even on powerpc, i see __die ends up calling show_regs_print_info().
>>>> Why print it twice?
>>>
>>> I don't understand what you mean, what is printed twice ?
>>>
>>> I can't see show_regs_print_info() printing the preemption model, am 
>>> I missing something ?
>>>
>>
>> Patch 2/9 add preemption string in dump_stack_print_info.
>>
>> __die -> show_regs() _> show_regs_print_info() -> 
>> dump_stack_print_info() -> init_utsname()->version, 
>> preempt_model_str(), BUILD_ID_VAL);
>>
>> Wont we end up in this path?
> 
> Indeed I missed that. You are right, we now get the information twice:

I think we can remove it from arch specific code and rely on lib/dump_stack?

And similar concern of printk vs pr_warn/pr_emerg would apply to that as 
well i guess.

> 
> [  440.068216] BUG: Unable to handle kernel data access on write at 
> 0xc09036fc

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Christophe Leroy]

Le 09/02/2025 à 15:38, Shrikanth Hegde a écrit :
> 
> 
> On 2/8/25 23:25, Christophe Leroy wrote:
>>
>>
>> Le 08/02/2025 à 14:42, Shrikanth Hegde a écrit :
>>>
>>>
>>> On 2/8/25 18:25, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 08/02/2025 à 08:35, Shrikanth Hegde a écrit :
>>>>>
>>>>>
>>>>> On 2/4/25 13:52, Sebastian Andrzej Siewior wrote:
>>>>>> Use preempt_model_str() instead of manually conducting the preemption
>>>>>> model. Use pr_emerg() instead of printk() to pass a loglevel.
>>>>>
>>>>> even on powerpc, i see __die ends up calling show_regs_print_info().
>>>>> Why print it twice?
>>>>
>>>> I don't understand what you mean, what is printed twice ?
>>>>
>>>> I can't see show_regs_print_info() printing the preemption model, am 
>>>> I missing something ?
>>>>
>>>
>>> Patch 2/9 add preemption string in dump_stack_print_info.
>>>
>>> __die -> show_regs() _> show_regs_print_info() -> 
>>> dump_stack_print_info() -> init_utsname()->version, 
>>> preempt_model_str(), BUILD_ID_VAL);
>>>
>>> Wont we end up in this path?
>>
>> Indeed I missed that. You are right, we now get the information twice:
> 
> I think we can remove it from arch specific code and rely on lib/ 
> dump_stack?

Yes I guess so.

> 
> And similar concern of printk vs pr_warn/pr_emerg would apply to that as 
> well i guess.

Well, powerpc's show_regs() calls it with 
show_regs_print_info(KERN_DEFAULT);

And dump_stack_print_info() uses printk with log_lvl so there should be 
no concern here.

> 
>>
>> [  440.068216] BUG: Unable to handle kernel data access on write at 
>> 0xc09036fc

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

[Sebastian Andrzej Siewior]

On 2025-02-08 13:31:27 [+0530], Shrikanth Hegde wrote:
> > 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.

Sorry, I can't follow. With PREEMPT_RT enabled, it will show einer
PREEMPT or PREEMPT_RT. It will add lazy if you pass preempt=lazy. It is
either 'full' or 'lazy'. 
You can't have VOLUNTARY+lazy or VOLUNTARY+RT.

What do I miss?

Sebastian

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Sebastian Andrzej Siewior]

On 2025-02-08 13:05:57 [+0530], Shrikanth Hegde wrote:
> 
> 
> On 2/4/25 13:52, Sebastian Andrzej Siewior wrote:
> > Use preempt_model_str() instead of manually conducting the preemption
> > model. Use pr_emerg() instead of printk() to pass a loglevel.
> 
> even on powerpc, i see __die ends up calling show_regs_print_info().
> Why print it twice?

Thank you for noticing. I did remove it on other architectures, I
somehow missed it here. Will remove it from from the arch code.

Sebastian

Re: [PATCH v2 3/9] arm: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

On 2025-02-03 15:16:26 [+0100], To linux-kernel@vger.kernel.org wrote:
> __die() invokes later __show_regs() -> show_regs_print_info() which
> prints the current preemption model.
> Remove it from the initial line.
> 
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Is it okay, to route this via the sched tree?

Sebastian

Re: [PATCH v2 4/9] arm64: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

On 2025-02-03 15:16:27 [+0100], To linux-kernel@vger.kernel.org wrote:
> __die() invokes later show_regs() -> show_regs_print_info() which prints
> the current preemption model.
> Remove it from the initial line.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Is it okay, to route this via the sched tree?

Sebastian

Re: [PATCH v2 7/9] x86: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

On 2025-02-03 15:16:30 [+0100], To linux-kernel@vger.kernel.org wrote:
> After __die_header(), __die_body() is always invoked. There we have
> show_regs() -> show_regs_print_info() which prints the current
> preemption model.
> Remove it from the initial line.
> 
> Cc: Josh Poimboeuf <jpoimboe@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: x86@kernel.org
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Is it okay, to route this via the sched tree?

Sebastian

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

[Shrikanth Hegde]

On 2/10/25 16:22, Sebastian Andrzej Siewior wrote:
> On 2025-02-08 13:31:27 [+0530], Shrikanth Hegde wrote:
>>> 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

[...]

>> 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.
> 
> Sorry, I can't follow. With PREEMPT_RT enabled, it will show einer
> PREEMPT or PREEMPT_RT. It will add lazy if you pass preempt=lazy. It is
> either 'full' or 'lazy'.
> You can't have VOLUNTARY+lazy or VOLUNTARY+RT.
> 
> What do I miss?

No No. That's not what i meant.

I was saying, as peter mentioned in the previous discussion, can we put 
something like below in changelog/comment for a easier understanding.?

"
RT+DYN:	 PREEMPT_{RT,(dyn_mode)}
RT+LAZY: PREEMPT_{RT,LAZY}
RT+FULL: PREEMPT_RT

DYN:       PREEMPT_(dyn_mode)
FULL:      PREEMPT
LAZY:      PREEMPTLAZY
VOLUNTARY: VOLUNTARY
NONE:      NONE
"

> 
> Sebastian

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Sebastian Andrzej Siewior]

On 2025-02-10 11:59:50 [+0100], To Shrikanth Hegde wrote:
> Thank you for noticing. I did remove it on other architectures, I
> somehow missed it here. Will remove it from from the arch code.

This is what I have for powerpc now. I'm going to repost the series,
currently waiting for arm/x86.

-------->8-----------

Subject: [PATCH] powerpc: Rely on generic printing of preemption model.

After the first printk in __die() there is show_regs() ->
show_regs_print_info() which prints the current
preemption model.

Remove the preempion model from the arch code.

Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Naveen N Rao <naveen@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/powerpc/kernel/traps.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index edf5cabe5dfdb..cb8e9357383e9 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -263,10 +263,9 @@ 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\n",
 	       IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) ? "LE" : "BE",
 	       PAGE_SIZE / 1024, get_mmu_str(),
-	       IS_ENABLED(CONFIG_PREEMPT) ? " PREEMPT" : "",
 	       IS_ENABLED(CONFIG_SMP) ? " SMP" : "",
 	       IS_ENABLED(CONFIG_SMP) ? (" NR_CPUS=" __stringify(NR_CPUS)) : "",
 	       debug_pagealloc_enabled() ? " DEBUG_PAGEALLOC" : "",
-- 
2.47.2

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

[Sebastian Andrzej Siewior]

On 2025-02-10 19:51:33 [+0530], Shrikanth Hegde wrote:
> No No. That's not what i meant.
> 
> I was saying, as peter mentioned in the previous discussion, can we put
> something like below in changelog/comment for a easier understanding.?

Ah. Okay, sure. I didn't realize anyone was asking but for this.

> "
> RT+DYN:	 PREEMPT_{RT,(dyn_mode)}
> RT+LAZY: PREEMPT_{RT,LAZY}
> RT+FULL: PREEMPT_RT
> 
> DYN:       PREEMPT_(dyn_mode)
> FULL:      PREEMPT
> LAZY:      PREEMPTLAZY
> VOLUNTARY: VOLUNTARY
> NONE:      NONE
> "

Sebastian

Re: [PATCH v2 4/9] arm64: Rely on generic printing of preemption model.

[Will Deacon]

On Mon, Feb 10, 2025 at 01:05:22PM +0100, Sebastian Andrzej Siewior wrote:
> On 2025-02-03 15:16:27 [+0100], To linux-kernel@vger.kernel.org wrote:
> > __die() invokes later show_regs() -> show_regs_print_info() which prints
> > the current preemption model.
> > Remove it from the initial line.
> > 
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> Is it okay, to route this via the sched tree?

Fine by me:

Acked-by: Will Deacon <will@kernel.org>

Will

Re: [PATCH v2 3/9] arm: Rely on generic printing of preemption model.

[Russell King (Oracle)]

On Mon, Feb 10, 2025 at 01:04:29PM +0100, Sebastian Andrzej Siewior wrote:
> On 2025-02-03 15:16:26 [+0100], To linux-kernel@vger.kernel.org wrote:
> > __die() invokes later __show_regs() -> show_regs_print_info() which
> > prints the current preemption model.
> > Remove it from the initial line.
> > 
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> Is it okay, to route this via the sched tree?

Sorry, its not obvious where show_regs_print_info() prints this.
dump_stack_print_info() itself doesn't directly. print_worker_info()
doesn't. print_stop_info() doesn't. Not sure whether print_scx_info()
does.

Maybe showing example output would help?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2 3/9] arm: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

On 2025-02-10 15:16:11 [+0000], Russell King (Oracle) wrote:
> On Mon, Feb 10, 2025 at 01:04:29PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2025-02-03 15:16:26 [+0100], To linux-kernel@vger.kernel.org wrote:
> > > __die() invokes later __show_regs() -> show_regs_print_info() which
> > > prints the current preemption model.
> > > Remove it from the initial line.
> > > 
> > > Cc: Russell King <linux@armlinux.org.uk>
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > 
> > Is it okay, to route this via the sched tree?
> 
> Sorry, its not obvious where show_regs_print_info() prints this.
> dump_stack_print_info() itself doesn't directly. print_worker_info()
> doesn't. print_stop_info() doesn't. Not sure whether print_scx_info()
> does.
> 
> Maybe showing example output would help?

Patch 2/9 adds this. [ https://lore.kernel.org/all/20250203141632.440554-3-bigeasy@linutronix.de/ ]

This this applied, die("test") on ARM ends as:

[    1.595106] Kernel panic - not syncing: test
[    1.596044] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.14.0-rc2-00009-gb80a798df08c-dirty #13 PREEMPT 
[    1.596768] Tainted: [W]=WARN
[    1.596946] Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 02/02/2022
[    1.597407] Call trace: 
[    1.598183]  unwind_backtrace from show_stack+0x10/0x14
[    1.599231]  show_stack from dump_stack_lvl+0x50/0x64
[    1.599492]  dump_stack_lvl from panic+0x114/0x358
[    1.599824]  panic from kernel_init+0x64/0x74
[    1.600048]  kernel_init from ret_from_fork+0x14/0x38
[    1.600349] Exception stack(0xdf829fb0 to 0xdf829ff8)
[    1.600730] 9fa0:                                     00000000 00000000 00000000 00000000
[    1.601102] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.601447] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    1.603163] ---[ end Kernel panic - not syncing: test ]---

Sebastian

Re: [PATCH v2 3/9] arm: Rely on generic printing of preemption model.

[Russell King (Oracle)]

On Mon, Feb 10, 2025 at 04:39:02PM +0100, Sebastian Andrzej Siewior wrote:
> On 2025-02-10 15:16:11 [+0000], Russell King (Oracle) wrote:
> > On Mon, Feb 10, 2025 at 01:04:29PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 2025-02-03 15:16:26 [+0100], To linux-kernel@vger.kernel.org wrote:
> > > > __die() invokes later __show_regs() -> show_regs_print_info() which
> > > > prints the current preemption model.
> > > > Remove it from the initial line.
> > > > 
> > > > Cc: Russell King <linux@armlinux.org.uk>
> > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > 
> > > Is it okay, to route this via the sched tree?
> > 
> > Sorry, its not obvious where show_regs_print_info() prints this.
> > dump_stack_print_info() itself doesn't directly. print_worker_info()
> > doesn't. print_stop_info() doesn't. Not sure whether print_scx_info()
> > does.
> > 
> > Maybe showing example output would help?
> 
> Patch 2/9 adds this. [ https://lore.kernel.org/all/20250203141632.440554-3-bigeasy@linutronix.de/ ]

That explains it - patch 2 is only sent to a very restricted subset
and not even to mailing lists that would be relevant in the absence
of a direct Cc. Ditto patch 1. All I received were patches 3 and 4.

In patch 1:

+	static char buf[128];
...
+		seq_buf_init(&s, buf, 128);

Why not sizeof(buf) ?

For patch 2:

"Use
pr_warn() instead of printk() to pass a loglevel. This makes it part of
generic WARN/ BUG traces.
"

How about cases which use dump_stack_lvl() to dump the output at a more
severe level than warning? Should the message be printed at a less
severe level than the other messages?

> This this applied, die("test") on ARM ends as:
> 
> [    1.595106] Kernel panic - not syncing: test
> [    1.596044] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W 6.14.0-rc2-00009-gb80a798df08c-dirty #13 PREEMPT
> [    1.596768] Tainted: [W]=WARN
> [    1.596946] Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 02/02/2022

Hmm. I've no idea what you're testing, what you've quoted makes zero
sense to me.

First...

void die(const char *str, struct pt_regs *regs, int err)

is the die function prototype, so it takes a bit more than what you've
indicated.

Second, "Kernel panic" suggests that panic() has been called. However,
this only happens when die() is called (or more specifically
oops_end()) from either interrupt context (in which case we get
"Kernel panic - Fatal exception in interrupt") or if panic_on_oops
is set ("Kernel panic - Fatal exception").

I don't see a path which would result in
"Kernel panic - not syncing: test" to be printed from this path.

Since __die() does not call dump_stack(), we're not going to call
dump_stack_print_info() from __die(), so I don't think it's appropriate
to remove this information.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v3 5/9] powerpc: Use preempt_model_str().

[Shrikanth Hegde]

On 2/10/25 19:53, Sebastian Andrzej Siewior wrote:
> On 2025-02-10 11:59:50 [+0100], To Shrikanth Hegde wrote:
>> Thank you for noticing. I did remove it on other architectures, I
>> somehow missed it here. Will remove it from from the arch code.
> 
> This is what I have for powerpc now. I'm going to repost the series,
> currently waiting for arm/x86.
> 
> -------->8-----------
> 
> Subject: [PATCH] powerpc: Rely on generic printing of preemption model.
> 
> After the first printk in __die() there is show_regs() ->
> show_regs_print_info() which prints the current
> preemption model.
> 
> Remove the preempion model from the arch code.
> 
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Naveen N Rao <naveen@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   arch/powerpc/kernel/traps.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index edf5cabe5dfdb..cb8e9357383e9 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -263,10 +263,9 @@ 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\n",
>   	       IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) ? "LE" : "BE",
>   	       PAGE_SIZE / 1024, get_mmu_str(),
> -	       IS_ENABLED(CONFIG_PREEMPT) ? " PREEMPT" : "",
>   	       IS_ENABLED(CONFIG_SMP) ? " SMP" : "",
>   	       IS_ENABLED(CONFIG_SMP) ? (" NR_CPUS=" __stringify(NR_CPUS)) : "",
>   	       debug_pagealloc_enabled() ? " DEBUG_PAGEALLOC" : "",

Looks good to me.

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

Re: [PATCH v2 3/9] arm: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

On 2025-02-10 16:26:56 [+0000], Russell King (Oracle) wrote:
> > Patch 2/9 adds this. [ https://lore.kernel.org/all/20250203141632.440554-3-bigeasy@linutronix.de/ ]
> 
> That explains it - patch 2 is only sent to a very restricted subset
> and not even to mailing lists that would be relevant in the absence
> of a direct Cc. Ditto patch 1. All I received were patches 3 and 4.
> 
> In patch 1:
> 
> +	static char buf[128];
> ...
> +		seq_buf_init(&s, buf, 128);
> 
> Why not sizeof(buf) ?

Indeed, why not. Updated.

> For patch 2:
> 
> "Use
> pr_warn() instead of printk() to pass a loglevel. This makes it part of
> generic WARN/ BUG traces.
> "

> How about cases which use dump_stack_lvl() to dump the output at a more
> severe level than warning? Should the message be printed at a less
> severe level than the other messages?

This is bad indeed. I reverted that piece.

> > This this applied, die("test") on ARM ends as:
> > 
> > [    1.595106] Kernel panic - not syncing: test
> > [    1.596044] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W 6.14.0-rc2-00009-gb80a798df08c-dirty #13 PREEMPT
> > [    1.596768] Tainted: [W]=WARN
> > [    1.596946] Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 02/02/2022
> 
> Hmm. I've no idea what you're testing, what you've quoted makes zero
> sense to me.
> 
> First...
> 
> void die(const char *str, struct pt_regs *regs, int err)
> 
> is the die function prototype, so it takes a bit more than what you've
> indicated.
> 
> Second, "Kernel panic" suggests that panic() has been called. However,
> this only happens when die() is called (or more specifically
> oops_end()) from either interrupt context (in which case we get
> "Kernel panic - Fatal exception in interrupt") or if panic_on_oops
> is set ("Kernel panic - Fatal exception").
> 
> I don't see a path which would result in
> "Kernel panic - not syncing: test" to be printed from this path.
> 
> Since __die() does not call dump_stack(), we're not going to call
> dump_stack_print_info() from __die(), so I don't think it's appropriate
> to remove this information.

Okay. Let me try again with a stack overflow during boot. With the
series:

| Freeing unused kernel image (initmem) memory: 2048K
| 8<--- cut here ---
| Unable to handle kernel paging request at virtual address df82a000 when write
| [df82a000] *pgd=80000040007003, *pmd=41487003, *pte=00000000
| Internal error: Oops: a07 [#1] SMP ARM
| Modules linked in:
| CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.14.0-rc2-00009-gdca8d546a8a3-dirty #16 PREEMPT
| Tainted: [W]=WARN
| Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 02/02/2022
| PC is at mmioset+0x38/0xac
| LR is at 0x34343434
| pc : [<c0a07a58>]    lr : [<34343434>]    psr: 20000013
| sp : df829c88  ip : df829ff4  fp : 00000000
| r10: 00000000  r9 : 00000000  r8 : 34343434
| r7 : 00000000  r6 : 00000000  r5 : c0a33278  r4 : c1207bd4
| r3 : 34343434  r2 : 00000080  r1 : 34343434  r0 : df829ea4
| Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
| Control: 30c5387d  Table: 40003000  DAC: 00000001
| Register r0 information: 2-page vmalloc region starting at 0xdf828000 allocated at kernel_clone+0x58/0x3b8
…
| Process swapper/0 (pid: 1, stack limit = 0x4c737b1e)
| Stack: (0xdf829c88 to 0xdf82a000)
…
| Call trace:
|  mmioset from func+0x24/0x50
|  func from kernel_init+0x7c/0x168
|  kernel_init from 0x34343434
| Code: e1a08001 e1a0e003 e2522040 a8ac410a (a8ac410a)
| ---[ end trace 0000000000000000 ]---
| Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
| ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---

Without:

| Freeing unused kernel image (initmem) memory: 2048K
| 8<--- cut here ---
| Unable to handle kernel paging request at virtual address df82a000 when write
| [df82a000] *pgd=80000040007003, *pmd=41487003, *pte=00000000
| Internal error: Oops: a07 [#1] PREEMPT SMP ARM
| Modules linked in:
| CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.14.0-rc2-00010-ged8f7288c355-dirty #17
| Tainted: [W]=WARN
| Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 02/02/2022
| PC is at mmioset+0x38/0xac
| LR is at 0x34343434
| pc : [<c0a079d8>]    lr : [<34343434>]    psr: 20000113
| sp : df829c88  ip : df829ff4  fp : 00000000
| r10: 00000000  r9 : 00000000  r8 : 34343434
| r7 : 00000000  r6 : 00000000  r5 : c0a331ec  r4 : c1207bd4
| r3 : 34343434  r2 : 00000080  r1 : 34343434  r0 : df829ea4
| Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
| Control: 30c5387d  Table: 40003000  DAC: 00000001
| Register r0 information: 2-page vmalloc region starting at 0xdf828000 allocated at kernel_clone+0x58/0x3b8
…
| Process swapper/0 (pid: 1, stack limit = 0x7d941701)
| Stack: (0xdf829c88 to 0xdf82a000)
…
| Call trace:
|  mmioset from func+0x24/0x50
|  func from kernel_init+0x7c/0x168
|  kernel_init from 0x34343434
| Code: e1a08001 e1a0e003 e2522040 a8ac410a (a8ac410a)
| ---[ end trace 0000000000000000 ]---
| Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
| ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---

As you see, the PREEMPT string moved and is still existing.

Sebastian

Re: [PATCH v2 3/9] arm: Rely on generic printing of preemption model.

[Russell King (Oracle)]

On Tue, Feb 11, 2025 at 03:52:49PM +0100, Sebastian Andrzej Siewior wrote:
> On 2025-02-10 16:26:56 [+0000], Russell King (Oracle) wrote:
> > > This this applied, die("test") on ARM ends as:
> > > 
> > > [    1.595106] Kernel panic - not syncing: test
> > > [    1.596044] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W 6.14.0-rc2-00009-gb80a798df08c-dirty #13 PREEMPT
> > > [    1.596768] Tainted: [W]=WARN
> > > [    1.596946] Hardware name: QEMU QEMU Virtual Machine, BIOS unknown 02/02/2022
> > 
> > Hmm. I've no idea what you're testing, what you've quoted makes zero
> > sense to me.
> > 
> > First...
> > 
> > void die(const char *str, struct pt_regs *regs, int err)
> > 
> > is the die function prototype, so it takes a bit more than what you've
> > indicated.
> > 
> > Second, "Kernel panic" suggests that panic() has been called. However,
> > this only happens when die() is called (or more specifically
> > oops_end()) from either interrupt context (in which case we get
> > "Kernel panic - Fatal exception in interrupt") or if panic_on_oops
> > is set ("Kernel panic - Fatal exception").
> > 
> > I don't see a path which would result in
> > "Kernel panic - not syncing: test" to be printed from this path.
> > 
> > Since __die() does not call dump_stack(), we're not going to call
> > dump_stack_print_info() from __die(), so I don't think it's appropriate
> > to remove this information.
> 
> Okay. Let me try again with a stack overflow during boot. With the
> series:
> 
> | Freeing unused kernel image (initmem) memory: 2048K
> | 8<--- cut here ---
> | Unable to handle kernel paging request at virtual address df82a000 when write
> | [df82a000] *pgd=80000040007003, *pmd=41487003, *pte=00000000
> | Internal error: Oops: a07 [#1] SMP ARM
> | Modules linked in:
> | CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Tainted: G        W          6.14.0-rc2-00009-gdca8d546a8a3-dirty #16 PREEMPT

Got it. Printed from __die() -> __show_regs() -> show_regs_print_info()
-> dump_stack_print_info().

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2 3/9] arm: Rely on generic printing of preemption model.

[Sebastian Andrzej Siewior]

On 2025-02-11 15:05:38 [+0000], Russell King (Oracle) wrote:
> 
> Got it. Printed from __die() -> __show_regs() -> show_regs_print_info()
> -> dump_stack_print_info().
> 
> Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thank you.

> Thanks!

Sebastian