v3 --> v4:
1. Move the declaration of function kallsyms_sym_address() to linux/kallsyms.h,
fix a build warning.
v2 --> v3:
1. Improve test cases, perform complete functional tests on functions
kallsyms_lookup_name(), kallsyms_on_each_symbol() and
kallsyms_on_each_match_symbol().
2. Add patch [PATCH v3 2/8] scripts/kallsyms: ensure that all possible
combinations are compressed.
3. The symbol type is not compressed regardless of whether
CONFIG_KALLSYMS_ALL is set or not. The memory overhead is increased
by less than 20KiB if CONFIG_KALLSYMS_ALL=n.
4. Discard [PATCH v2 3/8] kallsyms: Adjust the types of some local variables
v1 --> v2:
Add self-test facility
v1:
Currently, to search for a symbol, we need to expand the symbols in
'kallsyms_names' one by one, and then use the expanded string for
comparison. This is very slow.
In fact, we can first compress the name being looked up and then use
it for comparison when traversing 'kallsyms_names'.
This patch series optimizes the performance of function kallsyms_lookup_name(),
and function klp_find_object_symbol() in the livepatch module. Based on the
test results, the performance overhead is reduced to 5%. That is, the
performance of these functions is improved by 20 times.
To avoid increasing the kernel size in non-debug mode, the optimization is only
for the case CONFIG_KALLSYMS_ALL=y.
Zhen Lei (8):
scripts/kallsyms: rename build_initial_tok_table()
scripts/kallsyms: ensure that all possible combinations are compressed
scripts/kallsyms: don't compress symbol types
kallsyms: Improve the performance of kallsyms_lookup_name()
kallsyms: Add helper kallsyms_on_each_match_symbol()
livepatch: Use kallsyms_on_each_match_symbol() to improve performance
livepatch: Improve the search performance of
module_kallsyms_on_each_symbol()
kallsyms: Add self-test facility
include/linux/kallsyms.h | 9 +
init/Kconfig | 13 ++
kernel/Makefile | 1 +
kernel/kallsyms.c | 106 +++++++++-
kernel/kallsyms_selftest.c | 423 +++++++++++++++++++++++++++++++++++++
kernel/livepatch/core.c | 25 ++-
kernel/module/kallsyms.c | 13 +-
scripts/kallsyms.c | 24 ++-
8 files changed, 598 insertions(+), 16 deletions(-)
create mode 100644 kernel/kallsyms_selftest.c
--
2.25.1
Currently we traverse all symbols of all modules to find the specified
function for the specified module. But in reality, we just need to find
the given module and then traverse all the symbols in it.
In order to achieve this purpose, split the call to hook 'fn' into two
phases:
1. Finds the given module. Pass pointer 'mod'. Hook 'fn' directly returns
the comparison result of the module name without comparing the function
name.
2. Finds the given function in that module. Pass pointer 'mod = NULL'.
Hook 'fn' skip the comparison of module name and directly compare
function names.
Phase1: mod1-->mod2..(subsequent modules do not need to be compared)
|
Phase2: -->f1-->f2-->f3
Signed-off-by: Zhen Lei <[email protected]>
---
kernel/livepatch/core.c | 7 ++-----
kernel/module/kallsyms.c | 13 ++++++++++++-
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 31b57ccf908017e..98e23137e4133bc 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -130,15 +130,12 @@ static int klp_find_callback(void *data, const char *name,
{
struct klp_find_arg *args = data;
- if ((mod && !args->objname) || (!mod && args->objname))
- return 0;
+ if (mod)
+ return strcmp(args->objname, mod->name);
if (strcmp(args->name, name))
return 0;
- if (args->objname && strcmp(args->objname, mod->name))
- return 0;
-
args->addr = addr;
args->count++;
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index f5c5c9175333df7..b033613e6c7e3bb 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -510,6 +510,11 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
if (mod->state == MODULE_STATE_UNFORMED)
continue;
+ /* check mod->name first */
+ ret = fn(data, NULL, mod, 0);
+ if (ret)
+ continue;
+
/* Use rcu_dereference_sched() to remain compliant with the sparse tool */
preempt_disable();
kallsyms = rcu_dereference_sched(mod->kallsyms);
@@ -522,10 +527,16 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
continue;
ret = fn(data, kallsyms_symbol_name(kallsyms, i),
- mod, kallsyms_symbol_value(sym));
+ NULL, kallsyms_symbol_value(sym));
if (ret != 0)
goto out;
}
+
+ /*
+ * The given module is found, the subsequent modules do not
+ * need to be compared.
+ */
+ break;
}
out:
mutex_unlock(&module_mutex);
--
2.25.1
For a symbol, there may be more than one place that can be merged. For
example: nfs_fs_proc_net_init, there are two "f"+"s_" combinations.
And we're only compressing the first combination at the moment. Let's
compress all possible combinations.
Signed-off-by: Zhen Lei <[email protected]>
---
scripts/kallsyms.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 8caccc8f4a23703..3319d9f38d7a5f2 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -553,7 +553,7 @@ static void compress_symbols(const unsigned char *str, int idx)
unsigned char *p1, *p2;
for (i = 0; i < table_cnt; i++) {
-
+retry:
len = table[i]->len;
p1 = table[i]->sym;
@@ -585,6 +585,9 @@ static void compress_symbols(const unsigned char *str, int idx)
/* increase the counts for this symbol's new tokens */
learn_symbol(table[i]->sym, len);
+
+ /* May be more than one place that can be merged, try again */
+ goto retry;
}
}
--
2.25.1
Based on the test results of kallsyms_on_each_match_symbol() and
kallsyms_on_each_symbol(), the average performance can be improved by 20
to 30 times.
Signed-off-by: Zhen Lei <[email protected]>
---
kernel/livepatch/core.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 42f7e716d56bf72..31b57ccf908017e 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -153,6 +153,24 @@ static int klp_find_callback(void *data, const char *name,
return 0;
}
+static int klp_match_callback(void *data, unsigned long addr)
+{
+ struct klp_find_arg *args = data;
+
+ args->addr = addr;
+ args->count++;
+
+ /*
+ * Finish the search when the symbol is found for the desired position
+ * or the position is not defined for a non-unique symbol.
+ */
+ if ((args->pos && (args->count == args->pos)) ||
+ (!args->pos && (args->count > 1)))
+ return 1;
+
+ return 0;
+}
+
static int klp_find_object_symbol(const char *objname, const char *name,
unsigned long sympos, unsigned long *addr)
{
@@ -167,7 +185,7 @@ static int klp_find_object_symbol(const char *objname, const char *name,
if (objname)
module_kallsyms_on_each_symbol(klp_find_callback, &args);
else
- kallsyms_on_each_symbol(klp_find_callback, &args);
+ kallsyms_on_each_match_symbol(klp_match_callback, name, &args);
/*
* Ensure an address was found. If sympos is 0, ensure symbol is unique;
--
2.25.1
Added test cases for basic functions and performance of functions
kallsyms_lookup_name(), kallsyms_on_each_symbol() and
kallsyms_on_each_match_symbol(). It also calculates the compression rate
of the kallsyms compression algorithm for the current symbol set.
The basic functions test begins by testing a set of symbols whose address
values are known. Then, traverse all symbol addresses and find the
corresponding symbol name based on the address. It's impossible to
determine whether these addresses are correct, but we can use the above
three functions along with the addresses to test each other. Due to the
traversal operation of kallsyms_on_each_symbol() is too slow, only 60
symbols can be tested in one second, so let it test on average once
every 128 symbols. The other two functions validate all symbols.
If the basic functions test is passed, print only performance test
results. If the test fails, print error information, but do not perform
subsequent performance tests.
Start self-test automatically after system startup if
CONFIG_KALLSYMS_SELFTEST=y.
Example of output content: (prefix 'kallsyms_selftest:' is omitted)
start
---------------------------------------------------------
| nr_symbols | compressed size | original size | ratio(%) |
|---------------------------------------------------------|
| 174099 | 1960154 | 3750756 | 52.26 |
---------------------------------------------------------
kallsyms_lookup_name() looked up 174099 symbols
The time spent on each symbol is (ns): min=5250, max=726560, avg=302132
kallsyms_on_each_symbol() traverse all: 16659500 ns
kallsyms_on_each_match_symbol() traverse all: 557400 ns
finish
Signed-off-by: Zhen Lei <[email protected]>
---
include/linux/kallsyms.h | 1 +
init/Kconfig | 13 ++
kernel/Makefile | 1 +
kernel/kallsyms.c | 2 +-
kernel/kallsyms_selftest.c | 423 +++++++++++++++++++++++++++++++++++++
5 files changed, 439 insertions(+), 1 deletion(-)
create mode 100644 kernel/kallsyms_selftest.c
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index f9f2cc084cab16b..b46e1d8b0c18316 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -66,6 +66,7 @@ static inline void *dereference_symbol_descriptor(void *ptr)
}
#ifdef CONFIG_KALLSYMS
+unsigned long kallsyms_sym_address(int idx);
int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
unsigned long),
void *data);
diff --git a/init/Kconfig b/init/Kconfig
index 532362fcfe31fd3..60193fd185fb6e6 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1716,6 +1716,19 @@ config KALLSYMS
symbolic stack backtraces. This increases the size of the kernel
somewhat, as all symbols have to be loaded into the kernel image.
+config KALLSYMS_SELFTEST
+ bool "Test the basic functions and performance of kallsyms"
+ depends on KALLSYMS
+ default n
+ help
+ Test the basic functions and performance of some interfaces, such as
+ kallsyms_lookup_name. It also calculates the compression rate of the
+ kallsyms compression algorithm for the current symbol set.
+
+ Start self-test automatically after system startup. Suggest executing
+ "dmesg | grep kallsyms_selftest" to collect test results. "finish" is
+ displayed in the last line, indicating that the test is complete.
+
config KALLSYMS_ALL
bool "Include all symbols in kallsyms"
depends on DEBUG_KERNEL && KALLSYMS
diff --git a/kernel/Makefile b/kernel/Makefile
index 318789c728d3290..122a5fed457bd98 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -68,6 +68,7 @@ endif
obj-$(CONFIG_UID16) += uid16.o
obj-$(CONFIG_MODULE_SIG_FORMAT) += module_signature.o
obj-$(CONFIG_KALLSYMS) += kallsyms.o
+obj-$(CONFIG_KALLSYMS_SELFTEST) += kallsyms_selftest.o
obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
obj-$(CONFIG_CRASH_CORE) += crash_core.o
obj-$(CONFIG_KEXEC_CORE) += kexec_core.o
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index cbcc9c560f5c188..34e306eecbb60c8 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -193,7 +193,7 @@ static unsigned int get_symbol_offset(unsigned long pos)
return name - kallsyms_names;
}
-static unsigned long kallsyms_sym_address(int idx)
+unsigned long kallsyms_sym_address(int idx)
{
if (!IS_ENABLED(CONFIG_KALLSYMS_BASE_RELATIVE))
return kallsyms_addresses[idx];
diff --git a/kernel/kallsyms_selftest.c b/kernel/kallsyms_selftest.c
new file mode 100644
index 000000000000000..cf9751507e4337e
--- /dev/null
+++ b/kernel/kallsyms_selftest.c
@@ -0,0 +1,423 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Test the function and performance of kallsyms
+ *
+ * Copyright (C) Huawei Technologies Co., Ltd., 2022
+ *
+ * Authors: Zhen Lei <[email protected]> Huawei
+ */
+
+#define pr_fmt(fmt) "kallsyms_selftest: " fmt
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kallsyms.h>
+#include <linux/random.h>
+#include <linux/sched/clock.h>
+#include <linux/kthread.h>
+#include <linux/vmalloc.h>
+
+#include "kallsyms_internal.h"
+
+
+#define MAX_NUM_OF_RECORDS 64
+
+struct test_stat {
+ int min;
+ int max;
+ int save_cnt;
+ int real_cnt;
+ u64 sum;
+ char *name;
+ unsigned long addr;
+ unsigned long addrs[MAX_NUM_OF_RECORDS];
+};
+
+struct test_item {
+ char *name;
+ unsigned long addr;
+};
+
+#define ITEM_FUNC(s) \
+ { \
+ .name = #s, \
+ .addr = (unsigned long)s, \
+ }
+
+#define ITEM_DATA(s) \
+ { \
+ .name = #s, \
+ .addr = (unsigned long)&s, \
+ }
+
+static int test_var_bss_static;
+static int test_var_data_static = 1;
+int test_var_bss;
+int test_var_data = 1;
+
+static int test_func_static(void)
+{
+ test_var_bss_static++;
+ test_var_data_static++;
+
+ return 0;
+}
+
+int test_func(void)
+{
+ return test_func_static();
+}
+
+__weak int test_func_weak(void)
+{
+ test_var_bss++;
+ test_var_data++;
+ return 0;
+}
+
+static struct test_item test_items[] = {
+ ITEM_FUNC(test_func_static),
+ ITEM_FUNC(test_func),
+ ITEM_FUNC(test_func_weak),
+ ITEM_FUNC(vmalloc),
+ ITEM_FUNC(vfree),
+#ifdef CONFIG_KALLSYMS_ALL
+ ITEM_DATA(test_var_bss_static),
+ ITEM_DATA(test_var_data_static),
+ ITEM_DATA(test_var_bss),
+ ITEM_DATA(test_var_data),
+ ITEM_DATA(vmap_area_list),
+#endif
+};
+
+static char stub_name[KSYM_NAME_LEN];
+
+static int stat_symbol_len(void *data, const char *name,
+ struct module *mod, unsigned long addr)
+{
+ *(u32 *)data += strlen(name);
+
+ return 0;
+}
+
+static void test_kallsyms_compression_ratio(void)
+{
+ int i;
+ const u8 *name;
+ u32 pos;
+ u32 ratio, total_size, total_len = 0;
+
+ 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 = kallsyms_num_syms - 1;
+ name = &kallsyms_names[kallsyms_markers[pos >> 8]];
+ for (i = 0; i <= (pos & 0xff); i++)
+ name = name + (*name) + 1;
+
+ /*
+ * 1. The length fields is not counted
+ * 2. The memory occupied by array kallsyms_token_table[] and
+ * kallsyms_token_index[] needs to be counted.
+ */
+ total_size = (name - kallsyms_names) - kallsyms_num_syms;
+ pos = kallsyms_token_index[0xff];
+ total_size += pos + strlen(&kallsyms_token_table[pos]) + 1;
+ total_size += 0x100 * sizeof(u16);
+
+ pr_info(" ---------------------------------------------------------\n");
+ pr_info("| nr_symbols | compressed size | original size | ratio(%%) |\n");
+ pr_info("|---------------------------------------------------------|\n");
+ ratio = 10000ULL * total_size / total_len;
+ pr_info("| %10d | %10d | %10d | %2d.%-2d |\n",
+ kallsyms_num_syms, total_size, total_len, ratio / 100, ratio % 100);
+ 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, stat.sum / stat.real_cnt);
+}
+
+static int find_symbol(void *data, const char *name,
+ struct module *mod, unsigned long addr)
+{
+ struct test_stat *stat = (struct test_stat *)data;
+
+ if (strcmp(name, stat->name) == 0) {
+ stat->real_cnt++;
+ stat->addr = addr;
+
+ if (stat->save_cnt < MAX_NUM_OF_RECORDS) {
+ stat->addrs[stat->save_cnt] = addr;
+ stat->save_cnt++;
+ }
+
+ if (stat->real_cnt == stat->max)
+ return 1;
+ }
+
+ 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;
+ 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;
+
+ stat->real_cnt++;
+ stat->addr = addr;
+
+ if (stat->save_cnt < MAX_NUM_OF_RECORDS) {
+ stat->addrs[stat->save_cnt] = addr;
+ stat->save_cnt++;
+ }
+
+ if (stat->real_cnt == stat->max)
+ return 1;
+
+ 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;
+ int next = 0, nr_failed = 0;
+ char *prefix;
+ unsigned short rand;
+ unsigned long addr;
+ char namebuf[KSYM_NAME_LEN];
+ struct test_stat stat, stat1, stat2;
+
+ prefix = "kallsyms_lookup_name() for";
+ for (i = 0; i < ARRAY_SIZE(test_items); i++) {
+ addr = kallsyms_lookup_name(test_items[i].name);
+ if (addr != test_items[i].addr) {
+ nr_failed++;
+ pr_info("%s %s failed: addr=%lx, expect %lx\n",
+ prefix, test_items[i].name, addr, test_items[i].addr);
+ }
+ }
+
+ prefix = "kallsyms_on_each_symbol() for";
+ for (i = 0; i < ARRAY_SIZE(test_items); i++) {
+ memset(&stat, 0, sizeof(stat));
+ stat.max = INT_MAX;
+ stat.name = test_items[i].name;
+ kallsyms_on_each_symbol(find_symbol, &stat);
+ if (stat.addr != test_items[i].addr || stat.real_cnt != 1) {
+ nr_failed++;
+ pr_info("%s %s failed: count=%d, addr=%lx, expect %lx\n",
+ prefix, test_items[i].name,
+ stat.real_cnt, stat.addr, test_items[i].addr);
+ }
+ }
+
+ prefix = "kallsyms_on_each_match_symbol() for";
+ for (i = 0; i < ARRAY_SIZE(test_items); i++) {
+ memset(&stat, 0, sizeof(stat));
+ stat.max = INT_MAX;
+ stat.name = test_items[i].name;
+ kallsyms_on_each_match_symbol(match_symbol, test_items[i].name, &stat);
+ if (stat.addr != test_items[i].addr || stat.real_cnt != 1) {
+ nr_failed++;
+ pr_info("%s %s failed: count=%d, addr=%lx, expect %lx\n",
+ prefix, test_items[i].name,
+ stat.real_cnt, stat.addr, test_items[i].addr);
+ }
+ }
+
+ if (nr_failed)
+ return -EFAULT;
+
+ for (i = 0; i < kallsyms_num_syms; i++) {
+ addr = kallsyms_sym_address(i);
+ if (!is_ksym_addr(addr))
+ continue;
+
+ ret = lookup_symbol_name(addr, namebuf);
+ if (unlikely(ret)) {
+ namebuf[0] = 0;
+ goto failed;
+ }
+
+ stat.addr = kallsyms_lookup_name(namebuf);
+
+ memset(&stat1, 0, sizeof(stat1));
+ stat1.max = INT_MAX;
+ kallsyms_on_each_match_symbol(match_symbol, namebuf, &stat1);
+
+ /*
+ * kallsyms_on_each_symbol() is too slow, randomly select some
+ * symbols for test.
+ */
+ if (i >= next) {
+ memset(&stat2, 0, sizeof(stat2));
+ stat2.max = INT_MAX;
+ stat2.name = namebuf;
+ kallsyms_on_each_symbol(find_symbol, &stat2);
+
+ /*
+ * kallsyms_on_each_symbol() and kallsyms_on_each_match_symbol()
+ * need to get the same traversal result.
+ */
+ if (stat1.addr != stat2.addr ||
+ stat1.real_cnt != stat2.real_cnt ||
+ memcmp(stat1.addrs, stat2.addrs,
+ stat1.save_cnt * sizeof(stat1.addrs[0])))
+ goto failed;
+
+ /*
+ * The average of random increments is 128, that is, one of
+ * them is tested every 128 symbols.
+ */
+ get_random_bytes(&rand, sizeof(rand));
+ next = i + (rand & 0xff) + 1;
+ }
+
+ /* Need to be found at least once */
+ if (!stat1.real_cnt)
+ goto failed;
+
+ /*
+ * kallsyms_lookup_name() returns the address of the first
+ * symbol found and cannot be NULL.
+ */
+ if (!stat.addr || stat.addr != stat1.addrs[0])
+ goto failed;
+
+ /*
+ * If the addresses of all matching symbols are recorded, the
+ * target address needs to be exist.
+ */
+ if (stat1.real_cnt <= MAX_NUM_OF_RECORDS) {
+ for (j = 0; j < stat1.save_cnt; j++) {
+ if (stat1.addrs[j] == addr)
+ break;
+ }
+
+ if (j == stat1.save_cnt)
+ goto failed;
+ }
+ }
+
+ return 0;
+
+failed:
+ pr_info("Test for %dth symbol failed: (%s) addr=%lx", i, namebuf, addr);
+ return -EFAULT;
+}
+
+static int test_entry(void *p)
+{
+ int ret;
+
+ do {
+ schedule_timeout(5 * HZ);
+ } while (system_state != SYSTEM_RUNNING);
+
+ pr_info("start\n");
+ ret = test_kallsyms_basic_function();
+ if (ret) {
+ pr_info("abort\n");
+ return ret;
+ }
+
+ 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;
+}
+
+static int __init kallsyms_test_init(void)
+{
+ struct task_struct *t;
+
+ t = kthread_create(test_entry, NULL, "kallsyms_test");
+ if (IS_ERR(t)) {
+ pr_info("Create kallsyms selftest task failed\n");
+ return PTR_ERR(t);
+ }
+ kthread_bind(t, 0);
+ wake_up_process(t);
+
+ return 0;
+}
+late_initcall(kallsyms_test_init);
--
2.25.1
On Tue 2022-09-20 15:13:11, Zhen Lei wrote:
> For a symbol, there may be more than one place that can be merged. For
> example: nfs_fs_proc_net_init, there are two "f"+"s_" combinations.
> And we're only compressing the first combination at the moment.
Really?
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 8caccc8f4a23703..3319d9f38d7a5f2 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -553,7 +553,7 @@ static void compress_symbols(const unsigned char *str, int idx)
> unsigned char *p1, *p2;
>
> for (i = 0; i < table_cnt; i++) {
> -
> +retry:
> len = table[i]->len;
> p1 = table[i]->sym;
>
> @@ -585,6 +585,9 @@ static void compress_symbols(const unsigned char *str, int idx)
>
> /* increase the counts for this symbol's new tokens */
> learn_symbol(table[i]->sym, len);
> +
> + /* May be more than one place that can be merged, try again */
> + goto retry;
> }
> }
My understanding is that the code already tries to find the same
token several times. Here are the important parts of the existing
code:
static void compress_symbols(const unsigned char *str, int idx)
{
p2 = find_token(p1, len, str);
do {
/* replace the found token with idx */
*p2 = idx;
[...]
/* find the token on the symbol */
p2 = find_token(p1, size, str);
} while (p2);
Best Regards,
Petr
On 2022/9/21 16:00, Petr Mladek wrote:
> On Tue 2022-09-20 15:13:11, Zhen Lei wrote:
>> For a symbol, there may be more than one place that can be merged. For
>> example: nfs_fs_proc_net_init, there are two "f"+"s_" combinations.
>> And we're only compressing the first combination at the moment.
>
> Really?
Yes, there are about 200 such functions.
>
>> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
>> index 8caccc8f4a23703..3319d9f38d7a5f2 100644
>> --- a/scripts/kallsyms.c
>> +++ b/scripts/kallsyms.c
>> @@ -553,7 +553,7 @@ static void compress_symbols(const unsigned char *str, int idx)
>> unsigned char *p1, *p2;
>>
>> for (i = 0; i < table_cnt; i++) {
>> -
>> +retry:
>> len = table[i]->len;
>> p1 = table[i]->sym;
>>
>> @@ -585,6 +585,9 @@ static void compress_symbols(const unsigned char *str, int idx)
>>
>> /* increase the counts for this symbol's new tokens */
>> learn_symbol(table[i]->sym, len);
>> +
>> + /* May be more than one place that can be merged, try again */
>> + goto retry;
>> }
>> }
>
> My understanding is that the code already tries to find the same
> token several times. Here are the important parts of the existing
> code:
>
> static void compress_symbols(const unsigned char *str, int idx)
> {
>
> p2 = find_token(p1, len, str);
>
> do {
> /* replace the found token with idx */
> *p2 = idx;
> [...]
>
> /* find the token on the symbol */
> p2 = find_token(p1, size, str);
Oh, yes, it retries. Let me reanalyze it. However, the problem is
real, and there may be a problem somewhere in the loop.
>
> } while (p2);
>
> Best Regards,
> Petr
> .
>
--
Regards,
Zhen Lei
On 2022/9/21 16:31, Leizhen (ThunderTown) wrote:
>
>
> On 2022/9/21 16:00, Petr Mladek wrote:
>> On Tue 2022-09-20 15:13:11, Zhen Lei wrote:
>>> For a symbol, there may be more than one place that can be merged. For
>>> example: nfs_fs_proc_net_init, there are two "f"+"s_" combinations.
>>> And we're only compressing the first combination at the moment.
>>
>> Really?
>
> Yes, there are about 200 such functions.
>
>>
>>> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
>>> index 8caccc8f4a23703..3319d9f38d7a5f2 100644
>>> --- a/scripts/kallsyms.c
>>> +++ b/scripts/kallsyms.c
>>> @@ -553,7 +553,7 @@ static void compress_symbols(const unsigned char *str, int idx)
>>> unsigned char *p1, *p2;
>>>
>>> for (i = 0; i < table_cnt; i++) {
>>> -
>>> +retry:
>>> len = table[i]->len;
>>> p1 = table[i]->sym;
>>>
>>> @@ -585,6 +585,9 @@ static void compress_symbols(const unsigned char *str, int idx)
>>>
>>> /* increase the counts for this symbol's new tokens */
>>> learn_symbol(table[i]->sym, len);
>>> +
>>> + /* May be more than one place that can be merged, try again */
>>> + goto retry;
>>> }
>>> }
>>
>> My understanding is that the code already tries to find the same
>> token several times. Here are the important parts of the existing
>> code:
>>
>> static void compress_symbols(const unsigned char *str, int idx)
>> {
>>
>> p2 = find_token(p1, len, str);
>>
>> do {
>> /* replace the found token with idx */
>> *p2 = idx;
>> [...]
>>
>> /* find the token on the symbol */
>> p2 = find_token(p1, size, str);
>
> Oh, yes, it retries. Let me reanalyze it. However, the problem is
> real, and there may be a problem somewhere in the loop.
Hi, Petr:
Thanks. I found that it's my fault. The first round skip the type
character. But the next round will incorrectly skip one character,
so for nfs_fs_proc_net_init, the next round start from s, and using
^
the proposed "unsigned char type" in your next reply should solve
the problem. Thank you very much.
- for (i = 0; i < len - 1; i++)
+ for (i = sym_start_idx; i < len - 1; i++)
>
>>
>> } while (p2);
>>
>> Best Regards,
>> Petr
>> .
>>
>
--
Regards,
Zhen Lei