Re: [PATCH v3 4/4] panic: use sys_info_with_filter() to avoid duplicate backtraces

[Petr Mladek <pmladek@suse.com>]

On Thu 2026-07-02 19:13:26, Bradley Morgan wrote:
> On July 2, 2026 10:09:41 AM GMT+01:00, Petr Mladek <pmladek@suse.com>
> wrote:
> >On Mon 2026-06-29 13:54:18, Bradley Morgan wrote:
> >> On 29 June 2026 12:40:52 BST, Feng Tang <feng.tang@linux.alibaba.com>
> >> wrote:
> >> >On Fri, Jun 26, 2026 at 02:14:14PM +0200, Petr Mladek wrote:
> >> >> On Fri 2026-06-26 12:23:50, Petr Mladek wrote:
> >> >> > On Thu 2026-06-25 15:25:58, Bradley Morgan wrote:
> >> >> In watchdog, panic, and hung task detection scenarios, sys_info() can
> >> >> be called multiple times or alongside direct backtrace triggers like
> >> >> trigger_allbutcpu_cpu_backtrace(). This results in identical
> >backtraces
> >> >> being dumped repeatedly from all CPUs, cluttering the kernel log and
> >> >> delaying or obscuring critical debug details.
> >> 
> >> im feeling a new file to do all the force panic jazz, but putting tape
> >> on sys_info.c isn't bd either.
> >
> >I wonder how to move forward with this.
> >
> >Honestly, I am not sure what exactly you mean by creating another
> >API for tracking the reports so I could not judge it. Feel free
> >to sent some POC.
> 
> sup petr, here's my poc
> 
> This should make my entire thing make sense
> 
> >From eb587ed749ff5993c517f29799b369185c5ee7d8 Mon Sep 17 00:00:00 2001
> From: Bradley Morgan <include@grrlz.net>
> Date: Thu, 2 Jul 2026 18:09:23 +0000
> Subject: [POC] sys_info: Introduce incident state-tracking to prevent
>  duplicate diagnostics
> 
> In watchdog, panic, and hung task detection scenarios, sys_info()
> can be called multiple times or alongside direct debug output
> functions (like trigger_allbutcpu_cpu_backtrace(), print_modules(),
> print_irqtrace_events(), and dump_stack()). This leads to identical
> diagnostics and stack traces being dumped repeatedly, cluttering the
> kernel log and delaying critical panics.
> 
> Introduce a state tracking bitmask and helpers in a new file,
> lib/sys_info_filter.c:

New file suggests that it would implement an API using
sys_info_filter() prefix.

> - sys_info_filter_and_set(mask): Atomically tests which bits in a mask
>   have not yet been printed during the current incident, marks them as
>   printed, and returns that subset.

The name of the funtion is a kind of puzzle. I think that we
could do a better job.

> - sys_info_reset(): Clears the printed mask state.

This function has sys_info* prefix. It would expect it in sys_info.c

> Add SYS_INFO_MODULES, SYS_INFO_IRQTRACE, and SYS_INFO_STACK flags to
> include/linux/sys_info.h, and handle them inside sys_info's diagnostic
> dispatch.

I though about adding an information that we printed backtrace for this
CPU as well. But it not trivial. Different API shows different extra
info, like modules, IRQ backtrace, registers, code. I would leave
this complexity aside for now.

> Update the watchdogs, hung task detector, and panic core to call
> sys_info_filter_and_set() to deduplicate their diagnostic printouts, and
> sys_info_reset() when a warning incident concludes (e.g., when a stuck
> CPU recovers, or a new hung task check round begins).
> 
> This ensures each piece of system diagnostic is printed at most once per
> lockup/panic event, preventing console log spam.
> 
> Assisted-by: Gemini:gemini-3.5-flash
> Signed-off-by: Bradley Morgan <include@grrlz.net>

> --- /dev/null
> +++ b/lib/sys_info_filter.c
> @@ -0,0 +1,120 @@
> +static unsigned long sys_info_printed;
> +
> +unsigned long sys_info_filter_and_set(unsigned long si_mask)
> +{
> +	unsigned long old, new;
> +
> +	if (!si_mask)
> +		return 0;
> +
> +	do {
> +		old = READ_ONCE(sys_info_printed);
> +		if (!(si_mask & ~old))
> +			return 0;
> +		new = old | si_mask;
> +	} while (cmpxchg(&sys_info_printed, old, new) != old);

It is a good question whether to update the info using atomic
operations. One problem is that the mask is "unsigned long".
I am not sure if it natively atomic on all architectures.
32-bit architecures use extra locking when implementing
atomic operations with 64-bit values. And we should rather
avoid any locking in this code.

Well, long seems to be 32-bit on 32-bit x86 so it might be
safe after all.

> +void sys_info_reset(void)
> +static void __sys_info(unsigned long si_mask)
> +void sys_info(unsigned long si_mask)

I wonder why this sys_info*() API implementation has been moved
from sys_info.c to sys_info_filter.c.

I am sorry but I do not see any advantage in adding the new file
sys_info_filter.c

> NOTE!!: This is AI generated!! This **MAY** not be the finished product,
> this is ONLY the model!

IMHO, Gemini did pretty bad job in this case. Please, try to review
the AI generated before you send it. And send it only when you think
that it is reasonable enough. :-)

It is even fine to send "crap" but you should start the mail
with a warning that you send it just give us an idea what you
had it mind. And you should explain why you actually do not like.

Best Regards,
Petr