samples/bpf/Makefile | 1 + tools/testing/selftests/bpf/trace_helpers.c | 59 +++++++++++++++++---- 2 files changed, 49 insertions(+), 11 deletions(-)
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.
Acked-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Rong Tao <rongtao@cestc.cn>
---
v6: Apply libbpf_ensure_mem()
v5: https://lore.kernel.org/lkml/tencent_0E9E1A1C0981678D5E7EA9E4BDBA8EE2200A@qq.com/
Release the allocated memory once the load_kallsyms_refresh() upon error
given it's dynamically allocated.
v4: https://lore.kernel.org/lkml/tencent_59C74613113F0C728524B2A82FE5540A5E09@qq.com/
Make sure most cases we don't need the realloc() path to begin with,
and check strdup() return value.
v3: https://lore.kernel.org/lkml/tencent_50B4B2622FE7546A5FF9464310650C008509@qq.com/
Do not use structs and judge ksyms__add_symbol function return value.
v2: https://lore.kernel.org/lkml/tencent_B655EE5E5D463110D70CD2846AB3262EED09@qq.com/
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.com/
---
samples/bpf/Makefile | 1 +
tools/testing/selftests/bpf/trace_helpers.c | 59 +++++++++++++++++----
2 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 595b98d825ce..a49d0f759f5a 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -199,6 +199,7 @@ TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
TPROGS_CFLAGS += -I$(LIBBPF_INCLUDE)
TPROGS_CFLAGS += -I$(srctree)/tools/include
TPROGS_CFLAGS += -I$(srctree)/tools/perf
+TPROGS_CFLAGS += -I$(srctree)/tools/lib
TPROGS_CFLAGS += -DHAVE_ATTR_TEST=0
ifdef SYSROOT
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
index f83d9f65c65b..316a7874a12b 100644
--- a/tools/testing/selftests/bpf/trace_helpers.c
+++ b/tools/testing/selftests/bpf/trace_helpers.c
@@ -14,14 +14,48 @@
#include <linux/limits.h>
#include <libelf.h>
#include <gelf.h>
+#ifndef __must_check
+#define __must_check
+#endif
+#include "bpf/libbpf_internal.h"
#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 size_t sym_cap;
static int sym_cnt;
+static int ksyms__add_symbol(const char *name, unsigned long addr)
+{
+ void *tmp;
+
+ tmp = strdup(name);
+ if (!tmp)
+ return -ENOMEM;
+ syms[sym_cnt].addr = addr;
+ syms[sym_cnt].name = tmp;
+
+ sym_cnt++;
+
+ return 0;
+}
+
+static void ksyms__free(void)
+{
+ unsigned int i;
+
+ if (!syms)
+ return;
+
+ for (i = 0; i < sym_cnt; i++)
+ free(syms[i].name);
+ free(syms);
+ syms = NULL;
+ sym_cnt = 0;
+ sym_cap = 0;
+}
+
static int ksym_cmp(const void *p1, const void *p2)
{
return ((struct ksym *)p1)->addr - ((struct ksym *)p2)->addr;
@@ -33,9 +67,7 @@ int load_kallsyms_refresh(void)
char func[256], buf[256];
char symbol;
void *addr;
- int i = 0;
-
- sym_cnt = 0;
+ int ret;
f = fopen("/proc/kallsyms", "r");
if (!f)
@@ -46,17 +78,22 @@ 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 = libbpf_ensure_mem((void **) &syms, &sym_cap,
+ sizeof(struct ksym), sym_cnt + 1);
+ if (ret)
+ goto error;
+ ret = ksyms__add_symbol(func, (unsigned long)addr);
+ if (ret)
+ goto error;
}
fclose(f);
- sym_cnt = i;
qsort(syms, sym_cnt, sizeof(struct ksym), ksym_cmp);
return 0;
+
+error:
+ ksyms__free();
+ return ret;
}
int load_kallsyms(void)
--
2.39.3On Mon, Aug 21, 2023 at 02:45:20PM +0800, 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.
>
> Acked-by: Stanislav Fomichev <sdf@google.com>
> Signed-off-by: Rong Tao <rongtao@cestc.cn>
> ---
> v6: Apply libbpf_ensure_mem()
> v5: https://lore.kernel.org/lkml/tencent_0E9E1A1C0981678D5E7EA9E4BDBA8EE2200A@qq.com/
> Release the allocated memory once the load_kallsyms_refresh() upon error
> given it's dynamically allocated.
> v4: https://lore.kernel.org/lkml/tencent_59C74613113F0C728524B2A82FE5540A5E09@qq.com/
> Make sure most cases we don't need the realloc() path to begin with,
> and check strdup() return value.
> v3: https://lore.kernel.org/lkml/tencent_50B4B2622FE7546A5FF9464310650C008509@qq.com/
> Do not use structs and judge ksyms__add_symbol function return value.
> v2: https://lore.kernel.org/lkml/tencent_B655EE5E5D463110D70CD2846AB3262EED09@qq.com/
> 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.com/
> ---
> samples/bpf/Makefile | 1 +
> tools/testing/selftests/bpf/trace_helpers.c | 59 +++++++++++++++++----
> 2 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 595b98d825ce..a49d0f759f5a 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -199,6 +199,7 @@ TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> TPROGS_CFLAGS += -I$(LIBBPF_INCLUDE)
> TPROGS_CFLAGS += -I$(srctree)/tools/include
> TPROGS_CFLAGS += -I$(srctree)/tools/perf
> +TPROGS_CFLAGS += -I$(srctree)/tools/lib
> TPROGS_CFLAGS += -DHAVE_ATTR_TEST=0
>
> ifdef SYSROOT
> diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
> index f83d9f65c65b..316a7874a12b 100644
> --- a/tools/testing/selftests/bpf/trace_helpers.c
> +++ b/tools/testing/selftests/bpf/trace_helpers.c
> @@ -14,14 +14,48 @@
> #include <linux/limits.h>
> #include <libelf.h>
> #include <gelf.h>
> +#ifndef __must_check
> +#define __must_check
> +#endif
I think you need to fix this on samples/bpf side
I tried to play with the samples/bpf/ includes, but couldn't find a way to
make this work.. selftests base includes on tools/include, while samples
have $(objtree)/usr/include as first include and AFAICS the __must_check is
defined under __KERNEL__ ifdef
I guess the reason samples use $(objtree)/usr/include is to get some struct
definitions which are not in tools/include, but looks like some samples objects
already use vmlinux.h include, so that could be the way forward to fix that
or get rid of the trace_helpers.c or bpf/selftests dependency ;-)
> +#include "bpf/libbpf_internal.h"
>
> #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 size_t sym_cap;
> static int sym_cnt;
nit, sym_cnt should be size_t
jirka
>
> +static int ksyms__add_symbol(const char *name, unsigned long addr)
> +{
> + void *tmp;
> +
> + tmp = strdup(name);
> + if (!tmp)
> + return -ENOMEM;
> + syms[sym_cnt].addr = addr;
> + syms[sym_cnt].name = tmp;
> +
> + sym_cnt++;
> +
> + return 0;
> +}
> +
> +static void ksyms__free(void)
> +{
> + unsigned int i;
> +
> + if (!syms)
> + return;
> +
> + for (i = 0; i < sym_cnt; i++)
> + free(syms[i].name);
> + free(syms);
> + syms = NULL;
> + sym_cnt = 0;
> + sym_cap = 0;
> +}
> +
> static int ksym_cmp(const void *p1, const void *p2)
> {
> return ((struct ksym *)p1)->addr - ((struct ksym *)p2)->addr;
> @@ -33,9 +67,7 @@ int load_kallsyms_refresh(void)
> char func[256], buf[256];
> char symbol;
> void *addr;
> - int i = 0;
> -
> - sym_cnt = 0;
> + int ret;
>
> f = fopen("/proc/kallsyms", "r");
> if (!f)
> @@ -46,17 +78,22 @@ 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 = libbpf_ensure_mem((void **) &syms, &sym_cap,
> + sizeof(struct ksym), sym_cnt + 1);
> + if (ret)
> + goto error;
> + ret = ksyms__add_symbol(func, (unsigned long)addr);
> + if (ret)
> + goto error;
> }
> fclose(f);
> - sym_cnt = i;
> qsort(syms, sym_cnt, sizeof(struct ksym), ksym_cmp);
> return 0;
> +
> +error:
> + ksyms__free();
> + return ret;
> }
>
> int load_kallsyms(void)
> --
> 2.39.3
>
>
> I think you need to fix this on samples/bpf side > > I tried to play with the samples/bpf/ includes, but couldn't find a way to > make this work.. selftests base includes on tools/include, while samples > have $(objtree)/usr/include as first include and AFAICS the __must_check is > defined under __KERNEL__ ifdef > > I guess the reason samples use $(objtree)/usr/include is to get some struct > definitions which are not in tools/include, but looks like some samples objects > already use vmlinux.h include, so that could be the way forward to fix that I tried the method you suggested, and some unexpected problems occurred. Maybe, we can apply v5 [0] first, and then solve this problem? Best wishes, Rong Tao [0] v5: https://lore.kernel.org/lkml/tencent_0E9E1A1C0981678D5E7EA9E4BDBA8EE2200A@qq.com/
On Tue, Aug 22, 2023 at 08:38:58AM +0800, Rong Tao wrote: > > I think you need to fix this on samples/bpf side > > > > I tried to play with the samples/bpf/ includes, but couldn't find a way to > > make this work.. selftests base includes on tools/include, while samples > > have $(objtree)/usr/include as first include and AFAICS the __must_check is > > defined under __KERNEL__ ifdef > > > > I guess the reason samples use $(objtree)/usr/include is to get some struct > > definitions which are not in tools/include, but looks like some samples objects > > already use vmlinux.h include, so that could be the way forward to fix that > > I tried the method you suggested, and some unexpected problems occurred. Maybe, > we can apply v5 [0] first, and then solve this problem? how about change below, seem to work for me jirka --- diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index 151ec320818b..3115f054dca7 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -316,6 +316,8 @@ XDP_SAMPLE_CFLAGS += -Wall -O2 \ $(obj)/$(XDP_SAMPLE): TPROGS_CFLAGS = $(XDP_SAMPLE_CFLAGS) $(obj)/$(XDP_SAMPLE): $(src)/xdp_sample_user.h $(src)/xdp_sample_shared.h +$(obj)/$(TRACE_HELPERS): TPROGS_CFLAGS := $(TPROGS_CFLAGS) -D__must_check= + -include $(BPF_SAMPLES_PATH)/Makefile.target VMLINUX_BTF_PATHS ?= $(abspath $(if $(O),$(O)/vmlinux)) \ diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c index 316a7874a12b..551547ba6829 100644 --- a/tools/testing/selftests/bpf/trace_helpers.c +++ b/tools/testing/selftests/bpf/trace_helpers.c @@ -14,9 +14,6 @@ #include <linux/limits.h> #include <libelf.h> #include <gelf.h> -#ifndef __must_check -#define __must_check -#endif #include "bpf/libbpf_internal.h" #define TRACEFS_PIPE "/sys/kernel/tracing/trace_pipe"
© 2016 - 2025 Red Hat, Inc.