* [PATCH] bpf: make bpf_stackmap_copy conditionally called
@ 2016-03-10 1:43 Arnd Bergmann
2016-03-10 3:38 ` Alexei Starovoitov
0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2016-03-10 1:43 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Arnd Bergmann, David S. Miller, Daniel Borkmann, netdev, linux-kernel
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 <arnd@arndb.de>
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] bpf: make bpf_stackmap_copy conditionally called
2016-03-10 1:43 [PATCH] bpf: make bpf_stackmap_copy conditionally called Arnd Bergmann
@ 2016-03-10 3:38 ` Alexei Starovoitov
2016-03-10 9:11 ` Arnd Bergmann
0 siblings, 1 reply; 3+ messages in thread
From: Alexei Starovoitov @ 2016-03-10 3:38 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann, netdev,
linux-kernel
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 <arnd@arndb.de>
> 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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bpf: make bpf_stackmap_copy conditionally called
2016-03-10 3:38 ` Alexei Starovoitov
@ 2016-03-10 9:11 ` Arnd Bergmann
0 siblings, 0 replies; 3+ messages in thread
From: Arnd Bergmann @ 2016-03-10 9:11 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, David S. Miller, Daniel Borkmann, netdev,
linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-03-10 9:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-10 1:43 [PATCH] bpf: make bpf_stackmap_copy conditionally called Arnd Bergmann
2016-03-10 3:38 ` Alexei Starovoitov
2016-03-10 9:11 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).