[PATCH v3] kallsyms: Remove the performance test from kallsyms_selftest.c

* [PATCH v3] kallsyms: Remove the performance test from kallsyms_selftest.c
@ 2023-01-10 13:01 Zhen Lei
  2023-01-13 23:23 ` Luis Chamberlain
  0 siblings, 1 reply; 3+ messages in thread
From: Zhen Lei @ 2023-01-10 13:01 UTC (permalink / raw)
  To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Steven Rostedt, Masami Hiramatsu, Mark Rutland, bpf,
	linux-trace-kernel, live-patching, linux-kernel,
	Luis Chamberlain, linux-modules
  Cc: Zhen Lei

[T58] BUG: sleeping function called from invalid context at kernel/kallsyms.c:305
[T58] in_atomic(): 0, irqs_disabled(): 128, non_block: 0, pid: 58, name: kallsyms_test
[T58] preempt_count: 0, expected: 0
[T58] RCU nest depth: 0, expected: 0
[T58] no locks held by kallsyms_test/58.
[T58] irq event stamp: 18899904
[T58] hardirqs last enabled at (18899903): finish_task_switch.isra.0 (core.c:?)
[T58] hardirqs last disabled at (18899904): test_perf_kallsyms_on_each_symbol (kallsyms_selftest.c:?)
[T58] softirqs last enabled at (18899886): __do_softirq (??:?)
[T58] softirqs last disabled at (18899879): ____do_softirq (irq.c:?)
[T58] CPU: 0 PID: 58 Comm: kallsyms_test Tainted: G T  6.1.0-next-20221215 #2
[T58] Hardware name: linux,dummy-virt (DT)
[T58] Call trace:
[T58] dump_backtrace (??:?)
[T58] show_stack (??:?)
[T58] dump_stack_lvl (??:?)
[T58] dump_stack (??:?)
[T58] __might_resched (??:?)
[T58] kallsyms_on_each_symbol (??:?)
[T58] test_perf_kallsyms_on_each_symbol (kallsyms_selftest.c:?)
[T58] test_entry (kallsyms_selftest.c:?)
[T58] kthread (kthread.c:?)
[T58] ret_from_fork (??:?)
[T58] kallsyms_selftest: kallsyms_on_each_symbol() traverse all: 5744310840 ns
[T58] kallsyms_selftest: kallsyms_on_each_match_symbol() traverse all: 1164580 ns
[T58] kallsyms_selftest: finish

Functions kallsyms_on_each_symbol() and kallsyms_on_each_match_symbol()
call the user-registered hook function for each symbol that meets the
requirements. Because it is uncertain how long that hook function will
execute, they call cond_resched() to avoid consuming CPU resources for a
long time. Therefore, they cannot be called with irqs disabled.

Given that people don't care about the performance of kallsyms, let's
simply remove it.

Fixes: 30f3bb09778d ("kallsyms: Add self-test facility")
Reported-by: Anders Roxell <anders.roxell@linaro.org>
Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 kernel/kallsyms_selftest.c | 88 +-------------------------------------
 1 file changed, 1 insertion(+), 87 deletions(-)

v1 --> v2:
1. Keep calling cond_resched() when CONFIG_KALLSYMS_SELFTEST=y. Instead,
   function kallsyms_on_each_match_symbol() and kallsyms_on_each_symbol()
   are not protected by local_irq_save() in kallsyms_selftest.c.

v2 --> v3:
1. Remove the performance test functions.

[v2] https://lkml.org/lkml/2022/12/27/762

diff --git a/kernel/kallsyms_selftest.c b/kernel/kallsyms_selftest.c
index f35d9cc1aab1544..79c42d80b8f69a1 100644
--- a/kernel/kallsyms_selftest.c
+++ b/kernel/kallsyms_selftest.c
@@ -12,9 +12,9 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/kallsyms.h>
+#include <linux/kthread.h>
 #include <linux/random.h>
 #include <linux/sched/clock.h>
-#include <linux/kthread.h>
 #include <linux/vmalloc.h>
 
 #include "kallsyms_internal.h"
@@ -93,7 +93,6 @@ static struct test_item test_items[] = {
 #endif
 };
 
-static char stub_name[KSYM_NAME_LEN];
 
 static int stat_symbol_len(void *data, const char *name, struct module *mod, unsigned long addr)
 {
@@ -109,16 +108,6 @@ static void test_kallsyms_compression_ratio(void)
 
 	kallsyms_on_each_symbol(stat_symbol_len, &total_len);
 
-	/*
-	 * A symbol name cannot start with a number. This stub name helps us
-	 * traverse the entire symbol table without finding a match. It's used
-	 * for subsequent performance tests, and its length is the average
-	 * length of all symbol names.
-	 */
-	memset(stub_name, '4', sizeof(stub_name));
-	pos = total_len / kallsyms_num_syms;
-	stub_name[pos] = 0;
-
 	pos = 0;
 	num = 0;
 	off = 0;
@@ -154,43 +143,6 @@ static void test_kallsyms_compression_ratio(void)
 	pr_info(" ---------------------------------------------------------\n");
 }
 
-static int lookup_name(void *data, const char *name, struct module *mod, unsigned long addr)
-{
-	u64 t0, t1, t;
-	unsigned long flags;
-	struct test_stat *stat = (struct test_stat *)data;
-
-	local_irq_save(flags);
-	t0 = sched_clock();
-	(void)kallsyms_lookup_name(name);
-	t1 = sched_clock();
-	local_irq_restore(flags);
-
-	t = t1 - t0;
-	if (t < stat->min)
-		stat->min = t;
-
-	if (t > stat->max)
-		stat->max = t;
-
-	stat->real_cnt++;
-	stat->sum += t;
-
-	return 0;
-}
-
-static void test_perf_kallsyms_lookup_name(void)
-{
-	struct test_stat stat;
-
-	memset(&stat, 0, sizeof(stat));
-	stat.min = INT_MAX;
-	kallsyms_on_each_symbol(lookup_name, &stat);
-	pr_info("kallsyms_lookup_name() looked up %d symbols\n", stat.real_cnt);
-	pr_info("The time spent on each symbol is (ns): min=%d, max=%d, avg=%lld\n",
-		stat.min, stat.max, div_u64(stat.sum, stat.real_cnt));
-}
-
 static bool match_cleanup_name(const char *s, const char *name)
 {
 	char *p;
@@ -231,24 +183,6 @@ static int find_symbol(void *data, const char *name, struct module *mod, unsigne
 	return 0;
 }
 
-static void test_perf_kallsyms_on_each_symbol(void)
-{
-	u64 t0, t1;
-	unsigned long flags;
-	struct test_stat stat;
-
-	memset(&stat, 0, sizeof(stat));
-	stat.max = INT_MAX;
-	stat.name = stub_name;
-	stat.perf = 1;
-	local_irq_save(flags);
-	t0 = sched_clock();
-	kallsyms_on_each_symbol(find_symbol, &stat);
-	t1 = sched_clock();
-	local_irq_restore(flags);
-	pr_info("kallsyms_on_each_symbol() traverse all: %lld ns\n", t1 - t0);
-}
-
 static int match_symbol(void *data, unsigned long addr)
 {
 	struct test_stat *stat = (struct test_stat *)data;
@@ -267,23 +201,6 @@ static int match_symbol(void *data, unsigned long addr)
 	return 0;
 }
 
-static void test_perf_kallsyms_on_each_match_symbol(void)
-{
-	u64 t0, t1;
-	unsigned long flags;
-	struct test_stat stat;
-
-	memset(&stat, 0, sizeof(stat));
-	stat.max = INT_MAX;
-	stat.name = stub_name;
-	local_irq_save(flags);
-	t0 = sched_clock();
-	kallsyms_on_each_match_symbol(match_symbol, stat.name, &stat);
-	t1 = sched_clock();
-	local_irq_restore(flags);
-	pr_info("kallsyms_on_each_match_symbol() traverse all: %lld ns\n", t1 - t0);
-}
-
 static int test_kallsyms_basic_function(void)
 {
 	int i, j, ret;
@@ -460,9 +377,6 @@ static int test_entry(void *p)
 	}
 
 	test_kallsyms_compression_ratio();
-	test_perf_kallsyms_lookup_name();
-	test_perf_kallsyms_on_each_symbol();
-	test_perf_kallsyms_on_each_match_symbol();
 	pr_info("finish\n");
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread