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: <aFoFq7CJKGxZ-N-9@U-2FWC9VHC-2323.local>
Date: Tue, 24 Jun 2025 09:55:55 +0800
From: Feng Tang <feng.tang@...ux.alibaba.com>
To: Petr Mladek <pmladek@...e.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Lance Yang <lance.yang@...ux.dev>, Jonathan Corbet <corbet@....net>,
	linux-kernel@...r.kernel.org, paulmck@...nel.org,
	john.ogness@...utronix.de
Subject: Re: [PATCH V2 4/5] panic: add 'panic_sys_info=' setup option for
 sysctl and kernel cmdline

On Mon, Jun 23, 2025 at 05:04:46PM +0200, Petr Mladek wrote:
> On Mon 2025-06-16 09:08:39, Feng Tang wrote:
> > Add 'panic_sys_info=' setup which expects string like "tasks,mem,lock,...".
> 
> This patch actually adds also the sysctl interface. It should be
> mentioned in the "Subject" and here.
 
I mentioned 'sysctl' in the subjec line, but maybe it's not obvious :)

> That said, it might be better to add the sysctl interface in
> the previous patch and add just the setup() here.

OK.

> 
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -4541,6 +4541,19 @@
> >  			Use this option carefully, maybe worth to setup a
> >  			bigger log buffer with "log_buf_len" along with this.
> >  
> > +	panic_sys_info=
> 
> 
> > +			String of subsystem info to be dumped on panic.
> 
> I am not a native speaker but I have troubles to parse the above
> sentence. See below.
> > +			It expects string of comma-separated words like
> > +			"tasks,mem,timer,...", which is a human readable string
> > +			version of 'panic_print':
> > +			tasks: print all tasks info
> > +			mem: print system memory info
> > +			timer: print timer info
> > +			lock: print locks info if CONFIG_LOCKDEP is on
> > +			ftrace: print ftrace buffer
> > +			all_bt: print all CPUs backtrace (if available in the arch)
> > +			blocked_tasks: print only tasks in uninterruptible (blocked) state
> 
> This blob is hard to parse. I suggest to replace it with something
> like:
> 
> <proposal>
> 	panic_sys_info= A comma separated list of extra information to be dumped
> 			on panic.
> 			Format: val[,val...]
> 			Where @val can be any of the following:
> 
> 			tasks:		print all tasks info
> 			mem:		print system memory info
> 			timers:		print timers info
> 			locks:		print locks info if CONFIG_LOCKDEP is on
> 			ftrace:		print ftrace buffer
> 			all_bt:		print all CPUs backtrace (if available in the arch)
> 			blocked_tasks:	print only tasks in uninterruptible (blocked) state
> 
> 			This is a human readable alternative to the 'panic_print' option.
> </proposal>

Thanks! It's much better.

> > +
> >  	parkbd.port=	[HW] Parallel port number the keyboard adapter is
> >  			connected to, default is 0.
> >  			Format: <parport#>
> > diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> > index dd49a89a62d3..2013afd98605 100644
> > --- a/Documentation/admin-guide/sysctl/kernel.rst
> > +++ b/Documentation/admin-guide/sysctl/kernel.rst
> > @@ -899,6 +899,24 @@ So for example to print tasks and memory info on panic, user can::
> >    echo 3 > /proc/sys/kernel/panic_print
> >  
> >  
> > +panic_sys_info
> > +==============
> > +
> > +String of subsystem info to be dumped on panic. It expects string of
> 
> Same here.
> 
> > +comma-separated words like "tasks,mem,timer,...", which is a human
> > +readable string version of 'panic_print':
> 
> I would replace it with:
> 
> <proposal>
> A comma separated list of extra information to be dumped on panic,
> for example, "tasks,mem,timers,...".  It is a human readable alternative
> to 'panic_print'. Possible values are:
> </proposal>

Will change.

> > +
> > +=============   ===================================================
> > +tasks           print all tasks info
> > +mem             print system memory info
> > +timer           print timer info
> > +lock            print locks info if CONFIG_LOCKDEP is on
> > +ftrace          print ftrace buffer
> > +all_bt          print all CPUs backtrace (if available in the arch)
> > +blocked_tasks   print only tasks in uninterruptible (blocked) state
> > +=============   ===================================================
> > +
> > +
> >  panic_on_rcu_stall
> >  ==================
> 
> The rest looks good.
 
Thanks!

- Feng

> Best Regards,
> Petr

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ