Re: [PATCH] bpf: make bpf_stackmap_copy conditionally called
On Thursday 10 March 2016, Alexei Starovoitov wrote:
> yes. this is also ok-ish fix.
> I've sent different version already:
> http://patchwork.ozlabs.org/patch/595617/
>
> I considered the option like yours but it's relying on gcc doing
> dead code elimination of 'if (false) {}' branch and though kernel
> is never compiled with -O0. I didn't want to take the risk.
> I'm fine with either approach though.
Ok, and I see yours is already applied, so that's fine.
In general, I don't like __weak symbols in the kernel as they
make it less clear what is actually getting called, and I think my
version would have been safe, we rely on building with -O2 or -Os
in a lot of places because of similar things.
Arnd
Re: [PATCH] bpf: make bpf_stackmap_copy conditionally called
On Thu, Mar 10, 2016 at 02:43:42AM +0100, Arnd Bergmann wrote:
> Changing the bpf syscall to use the new bpf_stackmap_copy() helper for
> BPF_MAP_TYPE_STACK_TRACE causes a link error when CONFIG_PERF_EVENTS
> is disabled:
>
> kernel/built-in.o: In function `map_lookup_elem':
> :(.text+0x7fca4): undefined reference to `bpf_stackmap_copy'
>
> This patch simply avoids handling that case, which may or may not
> be the correct answer here.
>
> Signed-off-by: Arnd Bergmann
> Fixes: 557c0c6e7df8 ("bpf: convert stackmap to pre-allocation")
...
> - } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
> + } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE &&
> +IS_ENABLED(CONFIG_PERF_EVENTS)) {
> err = bpf_stackmap_copy(map, key, value);
> } else {
yes. this is also ok-ish fix.
I've sent different version already:
http://patchwork.ozlabs.org/patch/595617/
I considered the option like yours but it's relying on gcc doing
dead code elimination of 'if (false) {}' branch and though kernel
is never compiled with -O0. I didn't want to take the risk.
I'm fine with either approach though.
[PATCH] bpf: make bpf_stackmap_copy conditionally called
Changing the bpf syscall to use the new bpf_stackmap_copy() helper for
BPF_MAP_TYPE_STACK_TRACE causes a link error when CONFIG_PERF_EVENTS
is disabled:
kernel/built-in.o: In function `map_lookup_elem':
:(.text+0x7fca4): undefined reference to `bpf_stackmap_copy'
This patch simply avoids handling that case, which may or may not
be the correct answer here.
Signed-off-by: Arnd Bergmann
Fixes: 557c0c6e7df8 ("bpf: convert stackmap to pre-allocation")
---
kernel/bpf/syscall.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 2978d0d08869..b014b64b6116 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -290,7 +290,8 @@ static int map_lookup_elem(union bpf_attr *attr)
err = bpf_percpu_hash_copy(map, key, value);
} else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
err = bpf_percpu_array_copy(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
+ } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE &&
+ IS_ENABLED(CONFIG_PERF_EVENTS)) {
err = bpf_stackmap_copy(map, key, value);
} else {
rcu_read_lock();
--
2.7.0
