Bitmap definition for 'panic_print' is hard to remember and decode.
Add 'panic_sys_info='sysctl to take human readable string like
"tasks,mem,timers,locks,ftrace,..." and translate it into bitmap.

The detailed mapping is:
	SYS_INFO_TASKS		"tasks"
	SYS_INFO_MEM		"mem"
	SYS_INFO_TIMERS		"timers"
	SYS_INFO_LOCKS		"locks"
	SYS_INFO_FTRACE		"ftrace"
	SYS_INFO_ALL_CPU_BT	"all_bt"
	SYS_INFO_BLOCKED_TASKS	"blocked_tasks"

Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
 Documentation/admin-guide/sysctl/kernel.rst | 18 +++++
 include/linux/sys_info.h                    |  8 ++
 kernel/panic.c                              |  7 ++
 lib/sys_info.c                              | 90 +++++++++++++++++++++
 4 files changed, 123 insertions(+)

diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index 0d08b7a2db2d..cccb06d1a6bf 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
+==============
+
+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:
+
+=============   ===================================================
+tasks           print all tasks info
+mem             print system memory info
+timer           print timers 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
 ==================
 
diff --git a/include/linux/sys_info.h b/include/linux/sys_info.h
index 53b7e27dbf2a..89d77dc4f2ed 100644
--- a/include/linux/sys_info.h
+++ b/include/linux/sys_info.h
@@ -2,6 +2,8 @@
 #ifndef _LINUX_SYS_INFO_H
 #define _LINUX_SYS_INFO_H
 
+#include <linux/sysctl.h>
+
 /*
  * SYS_INFO_PANIC_CONSOLE_REPLAY is for panic case only, as it needs special
  * handling which only fits panic case.
@@ -16,5 +18,11 @@
 #define SYS_INFO_BLOCKED_TASKS		0x00000080
 
 void sys_info(unsigned long si_mask);
+unsigned long sys_info_parse_param(char *str);
 
+#ifdef CONFIG_SYSCTL
+int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
+					  void *buffer, size_t *lenp,
+					  loff_t *ppos);
+#endif
 #endif	/* _LINUX_SYS_INFO_H */
diff --git a/kernel/panic.c b/kernel/panic.c
index cbb0681177b3..d7aa427dc23c 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -126,6 +126,13 @@ static const struct ctl_table kern_panic_table[] = {
 		.mode           = 0644,
 		.proc_handler   = proc_douintvec,
 	},
+	{
+		.procname	= "panic_sys_info",
+		.data		= &panic_print,
+		.maxlen         = sizeof(panic_print),
+		.mode		= 0644,
+		.proc_handler	= sysctl_sys_info_handler,
+	},
 };
 
 static __init int kernel_panic_sysctls_init(void)
diff --git a/lib/sys_info.c b/lib/sys_info.c
index 53031e5cb98e..46d6f4f1ad2a 100644
--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -3,10 +3,100 @@
 #include <linux/console.h>
 #include <linux/kernel.h>
 #include <linux/ftrace.h>
+#include <linux/sysctl.h>
 #include <linux/nmi.h>
 
 #include <linux/sys_info.h>
 
+struct sys_info_name {
+	unsigned long bit;
+	const char *name;
+};
+
+/*
+ * When 'si_names' gets updated,  please make sure the 'sys_info_avail'
+ * below is updated accordingly.
+ */
+static const struct sys_info_name  si_names[] = {
+	{ SYS_INFO_TASKS,		"tasks" },
+	{ SYS_INFO_MEM,			"mem" },
+	{ SYS_INFO_TIMERS,		"timers" },
+	{ SYS_INFO_LOCKS,		"locks" },
+	{ SYS_INFO_FTRACE,		"ftrace" },
+	{ SYS_INFO_ALL_CPU_BT,		"all_bt" },
+	{ SYS_INFO_BLOCKED_TASKS,	"blocked_tasks" },
+};
+
+/* Expecting string like "xxx_sys_info=tasks,mem,timers,locks,ftrace,..." */
+unsigned long sys_info_parse_param(char *str)
+{
+	unsigned long si_bits = 0;
+	char *s, *name;
+	int i;
+
+	s = str;
+	while ((name = strsep(&s, ",")) && *name) {
+		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
+			if (!strcmp(name, si_names[i].name)) {
+				si_bits |= si_names[i].bit;
+				break;
+			}
+		}
+	}
+
+	return si_bits;
+}
+
+#ifdef CONFIG_SYSCTL
+
+static const char sys_info_avail[] = "tasks,mem,timers,locks,ftrace,all_bt,blocked_tasks";
+
+int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
+					  void *buffer, size_t *lenp,
+					  loff_t *ppos)
+{
+	char names[sizeof(sys_info_avail) + 1];
+	struct ctl_table table;
+	unsigned long *si_bits_global;
+
+	si_bits_global = ro_table->data;
+
+	if (write) {
+		unsigned long si_bits;
+		int ret;
+
+		table = *ro_table;
+		table.data = names;
+		table.maxlen = sizeof(names);
+		ret = proc_dostring(&table, write, buffer, lenp, ppos);
+		if (ret)
+			return ret;
+
+		si_bits = sys_info_parse_param(names);
+		/* The access to the global value is not synchronized. */
+		WRITE_ONCE(*si_bits_global, si_bits);
+		return 0;
+	} else {
+		/* for 'read' operation */
+		char *delim = "";
+		int i, len = 0;
+
+		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
+			if (*si_bits_global & si_names[i].bit) {
+				len += scnprintf(names + len, sizeof(names) - len,
+					"%s%s", delim, si_names[i].name);
+				delim = ",";
+			}
+		}
+
+		table = *ro_table;
+		table.data = names;
+		table.maxlen = sizeof(names);
+		return proc_dostring(&table, write, buffer, lenp, ppos);
+	}
+}
+#endif
+
 void sys_info(unsigned long si_mask)
 {
 	if (si_mask & SYS_INFO_TASKS)
-- 
2.43.5

On Thu 2025-07-03 10:10:02, Feng Tang wrote:
> Bitmap definition for 'panic_print' is hard to remember and decode.
> Add 'panic_sys_info='sysctl to take human readable string like
> "tasks,mem,timers,locks,ftrace,..." and translate it into bitmap.
> 
> The detailed mapping is:
> 	SYS_INFO_TASKS		"tasks"
> 	SYS_INFO_MEM		"mem"
> 	SYS_INFO_TIMERS		"timers"
> 	SYS_INFO_LOCKS		"locks"
> 	SYS_INFO_FTRACE		"ftrace"
> 	SYS_INFO_ALL_CPU_BT	"all_bt"
> 	SYS_INFO_BLOCKED_TASKS	"blocked_tasks"
> 
> --- a/lib/sys_info.c
> +++ b/lib/sys_info.c
> +static const char sys_info_avail[] = "tasks,mem,timers,locks,ftrace,all_bt,blocked_tasks";
> +
> +int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> +					  void *buffer, size_t *lenp,
> +					  loff_t *ppos)
> +{
> +	char names[sizeof(sys_info_avail) + 1];

The "+ 1" looks superfluous.

I guess that it is for the trailing '\0'. But sys_info_avail[] already
includes the trailing '\0' so it should be already counted by the sizeof().

Note that it would be needed with strlen(). But it should not be
needed with sizeof().

> +	struct ctl_table table;
> +	unsigned long *si_bits_global;
> +
> +	si_bits_global = ro_table->data;
> +
> +	if (write) {
> +		unsigned long si_bits;
> +		int ret;
> +
> +		table = *ro_table;
> +		table.data = names;
> +		table.maxlen = sizeof(names);
> +		ret = proc_dostring(&table, write, buffer, lenp, ppos);
> +		if (ret)
> +			return ret;
> +
> +		si_bits = sys_info_parse_param(names);
> +		/* The access to the global value is not synchronized. */
> +		WRITE_ONCE(*si_bits_global, si_bits);
> +		return 0;
> +	} else {
> +		/* for 'read' operation */
> +		char *delim = "";
> +		int i, len = 0;
> +

It looks to me that names[] can later be used non-initialized when
*si_bits_global == 0. We should initialized it here, something like:

		names[0] = '\0';

> +		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
> +			if (*si_bits_global & si_names[i].bit) {
> +				len += scnprintf(names + len, sizeof(names) - len,
> +					"%s%s", delim, si_names[i].name);
> +				delim = ",";
> +			}
> +		}
> +
> +		table = *ro_table;
> +		table.data = names;
> +		table.maxlen = sizeof(names);
> +		return proc_dostring(&table, write, buffer, lenp, ppos);
> +	}
> +}
> +#endif

Otherwise, it looks good.

Best Regards,
Petr

On Tue, Aug 12, 2025 at 12:23:07PM +0200, Petr Mladek wrote:
> On Thu 2025-07-03 10:10:02, Feng Tang wrote:
> > Bitmap definition for 'panic_print' is hard to remember and decode.
> > Add 'panic_sys_info='sysctl to take human readable string like
> > "tasks,mem,timers,locks,ftrace,..." and translate it into bitmap.
> > 
> > The detailed mapping is:
> > 	SYS_INFO_TASKS		"tasks"
> > 	SYS_INFO_MEM		"mem"
> > 	SYS_INFO_TIMERS		"timers"
> > 	SYS_INFO_LOCKS		"locks"
> > 	SYS_INFO_FTRACE		"ftrace"
> > 	SYS_INFO_ALL_CPU_BT	"all_bt"
> > 	SYS_INFO_BLOCKED_TASKS	"blocked_tasks"
> > 
> > --- a/lib/sys_info.c
> > +++ b/lib/sys_info.c
> > +static const char sys_info_avail[] = "tasks,mem,timers,locks,ftrace,all_bt,blocked_tasks";
> > +
> > +int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> > +					  void *buffer, size_t *lenp,
> > +					  loff_t *ppos)
> > +{
> > +	char names[sizeof(sys_info_avail) + 1];
> 
> The "+ 1" looks superfluous.
> 
> I guess that it is for the trailing '\0'. But sys_info_avail[] already
> includes the trailing '\0' so it should be already counted by the sizeof().
> 
> Note that it would be needed with strlen(). But it should not be
> needed with sizeof().

Yes, you are right. Will remove it.

> > +	struct ctl_table table;
> > +	unsigned long *si_bits_global;
> > +
> > +	si_bits_global = ro_table->data;
> > +
> > +	if (write) {
> > +		unsigned long si_bits;
> > +		int ret;
> > +
> > +		table = *ro_table;
> > +		table.data = names;
> > +		table.maxlen = sizeof(names);
> > +		ret = proc_dostring(&table, write, buffer, lenp, ppos);
> > +		if (ret)
> > +			return ret;
> > +
> > +		si_bits = sys_info_parse_param(names);
> > +		/* The access to the global value is not synchronized. */
> > +		WRITE_ONCE(*si_bits_global, si_bits);
> > +		return 0;
> > +	} else {
> > +		/* for 'read' operation */
> > +		char *delim = "";
> > +		int i, len = 0;
> > +
> 
> It looks to me that names[] can later be used non-initialized when
> *si_bits_global == 0. We should initialized it here, something like:
> 
> 		names[0] = '\0';

Good catch! Will add this.

> > +		for (i = 0; i < ARRAY_SIZE(si_names); i++) {
> > +			if (*si_bits_global & si_names[i].bit) {
> > +				len += scnprintf(names + len, sizeof(names) - len,
> > +					"%s%s", delim, si_names[i].name);
> > +				delim = ",";
> > +			}
> > +		}
> > +
> > +		table = *ro_table;
> > +		table.data = names;
> > +		table.maxlen = sizeof(names);
> > +		return proc_dostring(&table, write, buffer, lenp, ppos);
> > +	}
> > +}
> > +#endif
> 
> Otherwise, it looks good.
 
Thanks for reviewing the patchset!

- Feng

> Best Regards,
> Petr

[PATCH 3/5] panic: add 'panic_sys_info' sysctl to take human readable 
string parameter

Just a minor nitpick on the subject line: it seems to be missing the
"v3" tag, unlike the other patches in this series.

Thanks,
Lance

On Thu, Jul 03, 2025 at 10:56:36AM +0800, Lance Yang wrote:
> [PATCH 3/5] panic: add 'panic_sys_info' sysctl to take human readable string
> parameter
> 
> Just a minor nitpick on the subject line: it seems to be missing the
> "v3" tag, unlike the other patches in this series.

Thanks for the note!

Yes, I did some last minutes change and forgot adding the 'v3' :)


> Thanks,
> Lance