From: Rong Tao rongtao@cestc.cn
Static ksyms often have problems because the number of symbols exceeds the MAX_SYMS limit. Like changing the MAX_SYMS from 300000 to 400000 in commit e76a014334a6("selftests/bpf: Bump and validate MAX_SYMS") solves the problem somewhat, but it's not the perfect way.
This commit uses dynamic memory allocation, which completely solves the problem caused by the limitation of the number of kallsyms.
Signed-off-by: Rong Tao rongtao@cestc.cn --- v3: Do not use structs and judge ksyms__add_symbol function return value. v2: https://lore.kernel.org/lkml/tencent_B655EE5E5D463110D70CD2846AB3262EED09@qq... Do the usual len/capacity scheme here to amortize the cost of realloc, and don't free symbols. v1: https://lore.kernel.org/lkml/tencent_AB461510B10CD484E0B2F62E3754165F2909@qq... --- tools/testing/selftests/bpf/trace_helpers.c | 42 ++++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c index f83d9f65c65b..d8391a2122b4 100644 --- a/tools/testing/selftests/bpf/trace_helpers.c +++ b/tools/testing/selftests/bpf/trace_helpers.c @@ -18,10 +18,32 @@ #define TRACEFS_PIPE "/sys/kernel/tracing/trace_pipe" #define DEBUGFS_PIPE "/sys/kernel/debug/tracing/trace_pipe"
-#define MAX_SYMS 400000 -static struct ksym syms[MAX_SYMS]; +static struct ksym *syms; +static int sym_cap; static int sym_cnt;
+static int ksyms__add_symbol(const char *name, unsigned long addr) +{ + void *tmp; + unsigned int new_cap; + + if (sym_cnt + 1 > sym_cap) { + new_cap = sym_cap * 4 / 3; + tmp = realloc(syms, sizeof(struct ksym) * new_cap); + if (!tmp) + return -ENOMEM; + syms = tmp; + sym_cap = new_cap; + } + + syms[sym_cnt].addr = addr; + syms[sym_cnt].name = strdup(name); + + sym_cnt++; + + return 0; +} + static int ksym_cmp(const void *p1, const void *p2) { return ((struct ksym *)p1)->addr - ((struct ksym *)p2)->addr; @@ -33,9 +55,13 @@ int load_kallsyms_refresh(void) char func[256], buf[256]; char symbol; void *addr; - int i = 0; + int ret;
+ sym_cap = 1024; sym_cnt = 0; + syms = malloc(sizeof(struct ksym) * sym_cap); + if (!syms) + return -ENOMEM;
f = fopen("/proc/kallsyms", "r"); if (!f) @@ -46,15 +72,11 @@ int load_kallsyms_refresh(void) break; if (!addr) continue; - if (i >= MAX_SYMS) - return -EFBIG; - - syms[i].addr = (long) addr; - syms[i].name = strdup(func); - i++; + ret = ksyms__add_symbol(func, (unsigned long)addr); + if (ret) + return ret; } fclose(f); - sym_cnt = i; qsort(syms, sym_cnt, sizeof(struct ksym), ksym_cmp); return 0; }
On 08/12, Rong Tao wrote:
From: Rong Tao rongtao@cestc.cn
Static ksyms often have problems because the number of symbols exceeds the MAX_SYMS limit. Like changing the MAX_SYMS from 300000 to 400000 in commit e76a014334a6("selftests/bpf: Bump and validate MAX_SYMS") solves the problem somewhat, but it's not the perfect way.
This commit uses dynamic memory allocation, which completely solves the problem caused by the limitation of the number of kallsyms.
Signed-off-by: Rong Tao rongtao@cestc.cn
I believe this is the one that won the pw race: https://patchwork.kernel.org/project/netdevbpf/patch/tencent_50B4B2622FE7546...
Acked-by: Stanislav Fomichev sdf@google.com
v3: Do not use structs and judge ksyms__add_symbol function return value. v2: https://lore.kernel.org/lkml/tencent_B655EE5E5D463110D70CD2846AB3262EED09@qq... Do the usual len/capacity scheme here to amortize the cost of realloc, and don't free symbols. v1: https://lore.kernel.org/lkml/tencent_AB461510B10CD484E0B2F62E3754165F2909@qq...
tools/testing/selftests/bpf/trace_helpers.c | 42 ++++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c index f83d9f65c65b..d8391a2122b4 100644 --- a/tools/testing/selftests/bpf/trace_helpers.c +++ b/tools/testing/selftests/bpf/trace_helpers.c @@ -18,10 +18,32 @@ #define TRACEFS_PIPE "/sys/kernel/tracing/trace_pipe" #define DEBUGFS_PIPE "/sys/kernel/debug/tracing/trace_pipe" -#define MAX_SYMS 400000 -static struct ksym syms[MAX_SYMS]; +static struct ksym *syms; +static int sym_cap; static int sym_cnt; +static int ksyms__add_symbol(const char *name, unsigned long addr) +{
- void *tmp;
- unsigned int new_cap;
Nit: reverse Christmas tree, not sure we care for the tests though..
On 8/12/23 7:57 AM, Rong Tao wrote:
From: Rong Tao rongtao@cestc.cn
Static ksyms often have problems because the number of symbols exceeds the MAX_SYMS limit. Like changing the MAX_SYMS from 300000 to 400000 in commit e76a014334a6("selftests/bpf: Bump and validate MAX_SYMS") solves the problem somewhat, but it's not the perfect way.
This commit uses dynamic memory allocation, which completely solves the problem caused by the limitation of the number of kallsyms.
Signed-off-by: Rong Tao rongtao@cestc.cn
v3: Do not use structs and judge ksyms__add_symbol function return value. v2: https://lore.kernel.org/lkml/tencent_B655EE5E5D463110D70CD2846AB3262EED09@qq... Do the usual len/capacity scheme here to amortize the cost of realloc, and don't free symbols. v1: https://lore.kernel.org/lkml/tencent_AB461510B10CD484E0B2F62E3754165F2909@qq...
tools/testing/selftests/bpf/trace_helpers.c | 42 ++++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c index f83d9f65c65b..d8391a2122b4 100644 --- a/tools/testing/selftests/bpf/trace_helpers.c +++ b/tools/testing/selftests/bpf/trace_helpers.c @@ -18,10 +18,32 @@ #define TRACEFS_PIPE "/sys/kernel/tracing/trace_pipe" #define DEBUGFS_PIPE "/sys/kernel/debug/tracing/trace_pipe" -#define MAX_SYMS 400000 -static struct ksym syms[MAX_SYMS]; +static struct ksym *syms; +static int sym_cap; static int sym_cnt; +static int ksyms__add_symbol(const char *name, unsigned long addr) +{
- void *tmp;
- unsigned int new_cap;
- if (sym_cnt + 1 > sym_cap) {
new_cap = sym_cap * 4 / 3;
tmp = realloc(syms, sizeof(struct ksym) * new_cap);
if (!tmp)
return -ENOMEM;
syms = tmp;
sym_cap = new_cap;
- }
- syms[sym_cnt].addr = addr;
- syms[sym_cnt].name = strdup(name);
Fwiw, strdup() should error check too.. and for teardown in the test suite, lets also have the counterpart where we release all the allocated mem.
- sym_cnt++;
- return 0;
+}
- static int ksym_cmp(const void *p1, const void *p2) { return ((struct ksym *)p1)->addr - ((struct ksym *)p2)->addr;
@@ -33,9 +55,13 @@ int load_kallsyms_refresh(void) char func[256], buf[256]; char symbol; void *addr;
- int i = 0;
- int ret;
- sym_cap = 1024;
On my dev node, I have:
# cat /proc/kallsyms | wc -l 242586
Why starting out so low with 1k? I would have expected that for most cases we don't need the realloc() path to begin with, but just in corner cases like in e76a014334a6.
sym_cnt = 0;
- syms = malloc(sizeof(struct ksym) * sym_cap);
- if (!syms)
return -ENOMEM;
f = fopen("/proc/kallsyms", "r"); if (!f) @@ -46,15 +72,11 @@ int load_kallsyms_refresh(void) break; if (!addr) continue;
if (i >= MAX_SYMS)
return -EFBIG;
syms[i].addr = (long) addr;
syms[i].name = strdup(func);
i++;
ret = ksyms__add_symbol(func, (unsigned long)addr);
if (ret)
} fclose(f);return ret;
- sym_cnt = i; qsort(syms, sym_cnt, sizeof(struct ksym), ksym_cmp); return 0; }
Thanks, Daniel.
I just submit v4[0], make sure most cases we don't need the realloc() path to begin with, and check strdup() return value.
[0] https://lore.kernel.org/lkml/tencent_59C74613113F0C728524B2A82FE5540A5E09@qq...
Rong Tao
linux-kselftest-mirror@lists.linaro.org