From d79a3aeb747c17095d679cc4402d87f0e7c3405e Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 30 Oct 2025 12:44:17 +0100 Subject: panic: sys_info: capture si_bits_global before iterating over it Patch series "panic: sys_info: Refactor and fix a potential issue", v3. While targeting the compilation issue due to dangling variable, I have noticed more opportunities for refactoring that helps to avoid above mentioned compilation issue in a cleaner way and also fixes a potential problem with global variable access. This patch (of 6): The for-loop might re-read the content of the memory the si_bits_global points to on each iteration. Instead, just capture it for the sake of consistency and use that instead. Link: https://lkml.kernel.org/r/20251030132007.3742368-1-andriy.shevchenko@linux.intel.com Link: https://lkml.kernel.org/r/20251030132007.3742368-2-andriy.shevchenko@linux.intel.com Signed-off-by: Andy Shevchenko Reviewed-by: Feng Tang Reviewed-by: Petr Mladek Signed-off-by: Andrew Morton --- lib/sys_info.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'lib/sys_info.c') diff --git a/lib/sys_info.c b/lib/sys_info.c index 496f9151c9b6..d542a024406a 100644 --- a/lib/sys_info.c +++ b/lib/sys_info.c @@ -58,11 +58,11 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, char names[sizeof(sys_info_avail)]; struct ctl_table table; unsigned long *si_bits_global; + unsigned long si_bits; si_bits_global = ro_table->data; if (write) { - unsigned long si_bits; int ret; table = *ro_table; @@ -81,9 +81,12 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, char *delim = ""; int i, len = 0; + /* The access to the global value is not synchronized. */ + si_bits = READ_ONCE(*si_bits_global); + names[0] = '\0'; for (i = 0; i < ARRAY_SIZE(si_names); i++) { - if (*si_bits_global & si_names[i].bit) { + if (si_bits & si_names[i].bit) { len += scnprintf(names + len, sizeof(names) - len, "%s%s", delim, si_names[i].name); delim = ","; -- cgit v1.2.3 From 760fc597c33d5a727507c8bb19d6ab87a8c5885b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 30 Oct 2025 12:44:18 +0100 Subject: panic: sys_info: align constant definition names with parameters Align constant definition names with parameters to make it easier to map. It's also better to maintain and extend the names while keeping their uniqueness. Link: https://lkml.kernel.org/r/20251030132007.3742368-3-andriy.shevchenko@linux.intel.com Signed-off-by: Andy Shevchenko Reviewed-by: Feng Tang Reviewed-by: Petr Mladek Signed-off-by: Andrew Morton --- lib/sys_info.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/sys_info.c') diff --git a/lib/sys_info.c b/lib/sys_info.c index d542a024406a..6b0188b30227 100644 --- a/lib/sys_info.c +++ b/lib/sys_info.c @@ -23,7 +23,7 @@ static const struct sys_info_name si_names[] = { { SYS_INFO_TIMERS, "timers" }, { SYS_INFO_LOCKS, "locks" }, { SYS_INFO_FTRACE, "ftrace" }, - { SYS_INFO_ALL_CPU_BT, "all_bt" }, + { SYS_INFO_ALL_BT, "all_bt" }, { SYS_INFO_BLOCKED_TASKS, "blocked_tasks" }, }; @@ -118,7 +118,7 @@ void sys_info(unsigned long si_mask) if (si_mask & SYS_INFO_FTRACE) ftrace_dump(DUMP_ALL); - if (si_mask & SYS_INFO_ALL_CPU_BT) + if (si_mask & SYS_INFO_ALL_BT) trigger_all_cpu_backtrace(); if (si_mask & SYS_INFO_BLOCKED_TASKS) -- cgit v1.2.3 From d13adc6147f5848d6ad9900fdb1dbf9a280a2f64 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 30 Oct 2025 12:44:19 +0100 Subject: panic: sys_info:replace struct sys_info_name with plain array of strings There is no need to keep a custom structure just for the need of a plain array of strings. Replace struct sys_info_name with plain array of strings. With that done, simplify the code, in particular, naturally use for_each_set_bit() when iterating over si_bits_global bitmap. Link: https://lkml.kernel.org/r/20251030132007.3742368-4-andriy.shevchenko@linux.intel.com Signed-off-by: Andy Shevchenko Reviewed-by: Petr Mladek Cc: Feng Tang Signed-off-by: Andrew Morton --- lib/sys_info.c | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) (limited to 'lib/sys_info.c') diff --git a/lib/sys_info.c b/lib/sys_info.c index 6b0188b30227..29a63238b31d 100644 --- a/lib/sys_info.c +++ b/lib/sys_info.c @@ -1,30 +1,29 @@ // SPDX-License-Identifier: GPL-2.0-only -#include +#include #include +#include #include #include -#include #include +#include +#include +#include #include -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_BT, "all_bt" }, - { SYS_INFO_BLOCKED_TASKS, "blocked_tasks" }, +static const char * const si_names[] = { + [ilog2(SYS_INFO_TASKS)] = "tasks", + [ilog2(SYS_INFO_MEM)] = "mem", + [ilog2(SYS_INFO_TIMERS)] = "timers", + [ilog2(SYS_INFO_LOCKS)] = "locks", + [ilog2(SYS_INFO_FTRACE)] = "ftrace", + [ilog2(SYS_INFO_PANIC_CONSOLE_REPLAY)] = "", + [ilog2(SYS_INFO_ALL_BT)] = "all_bt", + [ilog2(SYS_INFO_BLOCKED_TASKS)] = "blocked_tasks", }; /* Expecting string like "xxx_sys_info=tasks,mem,timers,locks,ftrace,..." */ @@ -36,12 +35,9 @@ unsigned long sys_info_parse_param(char *str) 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; - } - } + i = match_string(si_names, ARRAY_SIZE(si_names), name); + if (i >= 0) + __set_bit(i, &si_bits); } return si_bits; @@ -85,10 +81,10 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, si_bits = READ_ONCE(*si_bits_global); names[0] = '\0'; - for (i = 0; i < ARRAY_SIZE(si_names); i++) { - if (si_bits & si_names[i].bit) { + for_each_set_bit(i, &si_bits, ARRAY_SIZE(si_names)) { + if (*si_names[i]) { len += scnprintf(names + len, sizeof(names) - len, - "%s%s", delim, si_names[i].name); + "%s%s", delim, si_names[i]); delim = ","; } } -- cgit v1.2.3 From eb72c4667f4567a7363f6e00d082d2ab32b6a03a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 30 Oct 2025 12:44:20 +0100 Subject: panic: sys_info: rewrite a fix for a compilation error (`make W=1`) Compiler was not happy about dead variable in use: lib/sys_info.c:52:19: error: variable 'sys_info_avail' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration] 52 | static const char sys_info_avail[] = "tasks,mem,timers,locks,ftrace,all_bt,blocked_tasks"; | ^~~~~~~~~~~~~~ This was fixed by adding __maybe_unused attribute that just hides the issue and didn't actually fix the root cause. Rewrite the fix by moving the local variable from stack to a heap. As a side effect this drops unneeded "synchronisation" of duplicative info and also makes code ready for the further refactoring. Link: https://lkml.kernel.org/r/20251030132007.3742368-5-andriy.shevchenko@linux.intel.com Signed-off-by: Andy Shevchenko Reviewed-by: Petr Mladek Cc: Feng Tang Signed-off-by: Andrew Morton --- lib/sys_info.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) (limited to 'lib/sys_info.c') diff --git a/lib/sys_info.c b/lib/sys_info.c index 29a63238b31d..eb5c1226bfc8 100644 --- a/lib/sys_info.c +++ b/lib/sys_info.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only +#include #include +#include #include #include #include @@ -11,10 +13,6 @@ #include -/* - * When 'si_names' gets updated, please make sure the 'sys_info_avail' - * below is updated accordingly. - */ static const char * const si_names[] = { [ilog2(SYS_INFO_TASKS)] = "tasks", [ilog2(SYS_INFO_MEM)] = "mem", @@ -45,25 +43,32 @@ unsigned long sys_info_parse_param(char *str) #ifdef CONFIG_SYSCTL -static const char sys_info_avail[] __maybe_unused = "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)]; struct ctl_table table; unsigned long *si_bits_global; unsigned long si_bits; + unsigned int i; + size_t maxlen; si_bits_global = ro_table->data; + maxlen = 0; + for (i = 0; i < ARRAY_SIZE(si_names); i++) + maxlen += strlen(si_names[i]) + 1; + + char *names __free(kfree) = kzalloc(maxlen, GFP_KERNEL); + if (!names) + return -ENOMEM; + if (write) { int ret; table = *ro_table; table.data = names; - table.maxlen = sizeof(names); + table.maxlen = maxlen; ret = proc_dostring(&table, write, buffer, lenp, ppos); if (ret) return ret; @@ -74,16 +79,15 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, return 0; } else { /* for 'read' operation */ + unsigned int len = 0; char *delim = ""; - int i, len = 0; /* The access to the global value is not synchronized. */ si_bits = READ_ONCE(*si_bits_global); - names[0] = '\0'; for_each_set_bit(i, &si_bits, ARRAY_SIZE(si_names)) { if (*si_names[i]) { - len += scnprintf(names + len, sizeof(names) - len, + len += scnprintf(names + len, maxlen - len, "%s%s", delim, si_names[i]); delim = ","; } @@ -91,7 +95,7 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, table = *ro_table; table.data = names; - table.maxlen = sizeof(names); + table.maxlen = maxlen; return proc_dostring(&table, write, buffer, lenp, ppos); } } -- cgit v1.2.3 From f791dcc842cb1cb3777ae4122be4cd37624ad53d Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 30 Oct 2025 12:44:21 +0100 Subject: panic: sys_info: deduplicate local variable 'table; assignments The both handlers use the local 'table' variable and assign the same data to it, deduplicate that. Link: https://lkml.kernel.org/r/20251030132007.3742368-6-andriy.shevchenko@linux.intel.com Signed-off-by: Andy Shevchenko Reviewed-by: Feng Tang Reviewed-by: Petr Mladek Signed-off-by: Andrew Morton --- lib/sys_info.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'lib/sys_info.c') diff --git a/lib/sys_info.c b/lib/sys_info.c index eb5c1226bfc8..94526de8482b 100644 --- a/lib/sys_info.c +++ b/lib/sys_info.c @@ -63,12 +63,13 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, if (!names) return -ENOMEM; + table = *ro_table; + table.data = names; + table.maxlen = maxlen; + if (write) { int ret; - table = *ro_table; - table.data = names; - table.maxlen = maxlen; ret = proc_dostring(&table, write, buffer, lenp, ppos); if (ret) return ret; @@ -93,9 +94,6 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, } } - table = *ro_table; - table.data = names; - table.maxlen = maxlen; return proc_dostring(&table, write, buffer, lenp, ppos); } } -- cgit v1.2.3 From 9125163273f8033af5d38907b483c1d9f99d781b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 30 Oct 2025 12:44:22 +0100 Subject: panic: sys_info: factor out read and write handlers For the sake of the code readability and easier maintenance factor out read and write sys_info handlers. [akpm@linux-foundation.org: coding-style cleanups] Link: https://lkml.kernel.org/r/20251030132007.3742368-7-andriy.shevchenko@linux.intel.com Signed-off-by: Andy Shevchenko Reviewed-by: Petr Mladek Cc: Feng Tang Signed-off-by: Andrew Morton --- lib/sys_info.c | 79 ++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 46 insertions(+), 33 deletions(-) (limited to 'lib/sys_info.c') diff --git a/lib/sys_info.c b/lib/sys_info.c index 94526de8482b..323624093e54 100644 --- a/lib/sys_info.c +++ b/lib/sys_info.c @@ -43,18 +43,56 @@ unsigned long sys_info_parse_param(char *str) #ifdef CONFIG_SYSCTL +static int sys_info_write_handler(const struct ctl_table *table, + void *buffer, size_t *lenp, loff_t *ppos, + unsigned long *si_bits_global) +{ + unsigned long si_bits; + int ret; + + ret = proc_dostring(table, 1, buffer, lenp, ppos); + if (ret) + return ret; + + si_bits = sys_info_parse_param(table->data); + + /* The access to the global value is not synchronized. */ + WRITE_ONCE(*si_bits_global, si_bits); + + return 0; +} + +static int sys_info_read_handler(const struct ctl_table *table, + void *buffer, size_t *lenp, loff_t *ppos, + unsigned long *si_bits_global) +{ + unsigned long si_bits; + unsigned int len = 0; + char *delim = ""; + unsigned int i; + + /* The access to the global value is not synchronized. */ + si_bits = READ_ONCE(*si_bits_global); + + for_each_set_bit(i, &si_bits, ARRAY_SIZE(si_names)) { + if (*si_names[i]) { + len += scnprintf(table->data + len, table->maxlen - len, + "%s%s", delim, si_names[i]); + delim = ","; + } + } + + return proc_dostring(table, 0, buffer, lenp, ppos); +} + int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, void *buffer, size_t *lenp, loff_t *ppos) { struct ctl_table table; - unsigned long *si_bits_global; - unsigned long si_bits; unsigned int i; size_t maxlen; - si_bits_global = ro_table->data; - maxlen = 0; for (i = 0; i < ARRAY_SIZE(si_names); i++) maxlen += strlen(si_names[i]) + 1; @@ -67,35 +105,10 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, table.data = names; table.maxlen = maxlen; - if (write) { - int ret; - - 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 */ - unsigned int len = 0; - char *delim = ""; - - /* The access to the global value is not synchronized. */ - si_bits = READ_ONCE(*si_bits_global); - - for_each_set_bit(i, &si_bits, ARRAY_SIZE(si_names)) { - if (*si_names[i]) { - len += scnprintf(names + len, maxlen - len, - "%s%s", delim, si_names[i]); - delim = ","; - } - } - - return proc_dostring(&table, write, buffer, lenp, ppos); - } + if (write) + return sys_info_write_handler(&table, buffer, lenp, ppos, ro_table->data); + else + return sys_info_read_handler(&table, buffer, lenp, ppos, ro_table->data); } #endif -- cgit v1.2.3 From 03ef32d665e8a23d7ce5965b8b035666cfb47866 Mon Sep 17 00:00:00 2001 From: Feng Tang Date: Thu, 13 Nov 2025 19:10:39 +0800 Subject: sys_info: add a default kernel sys_info mask Which serves as a global default sys_info mask. When users want the same system information for many error cases (panic, hung, lockup ...), they can chose to set this global knob only once, while not setting up each individual sys_info knobs. This just adds a 'lazy' option, and doesn't change existing kernel behavior as the mask is 0 by default. Link: https://lkml.kernel.org/r/20251113111039.22701-5-feng.tang@linux.alibaba.com Suggested-by: Andrew Morton Signed-off-by: Feng Tang Cc: Jonathan Corbet Cc: Lance Yang Cc: "Paul E . McKenney" Cc: Petr Mladek Cc: Steven Rostedt Signed-off-by: Andrew Morton --- lib/sys_info.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) (limited to 'lib/sys_info.c') diff --git a/lib/sys_info.c b/lib/sys_info.c index 323624093e54..f32a06ec9ed4 100644 --- a/lib/sys_info.c +++ b/lib/sys_info.c @@ -24,6 +24,13 @@ static const char * const si_names[] = { [ilog2(SYS_INFO_BLOCKED_TASKS)] = "blocked_tasks", }; +/* + * Default kernel sys_info mask. + * If a kernel module calls sys_info() with "parameter == 0", then + * this mask will be used. + */ +static unsigned long kernel_si_mask; + /* Expecting string like "xxx_sys_info=tasks,mem,timers,locks,ftrace,..." */ unsigned long sys_info_parse_param(char *str) { @@ -110,9 +117,26 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write, else return sys_info_read_handler(&table, buffer, lenp, ppos, ro_table->data); } + +static const struct ctl_table sys_info_sysctls[] = { + { + .procname = "kernel_sys_info", + .data = &kernel_si_mask, + .maxlen = sizeof(kernel_si_mask), + .mode = 0644, + .proc_handler = sysctl_sys_info_handler, + }, +}; + +static int __init sys_info_sysctl_init(void) +{ + register_sysctl_init("kernel", sys_info_sysctls); + return 0; +} +subsys_initcall(sys_info_sysctl_init); #endif -void sys_info(unsigned long si_mask) +static void __sys_info(unsigned long si_mask) { if (si_mask & SYS_INFO_TASKS) show_state(); @@ -135,3 +159,8 @@ void sys_info(unsigned long si_mask) if (si_mask & SYS_INFO_BLOCKED_TASKS) show_state_filter(TASK_UNINTERRUPTIBLE); } + +void sys_info(unsigned long si_mask) +{ + __sys_info(si_mask ? : kernel_si_mask); +} -- cgit v1.2.3