* Linux 6.1-rc3 build fail in include/linux/bpf.h @ 2022-10-31 11:14 David Laight 2022-10-31 12:03 ` David Laight 2022-10-31 12:17 ` Jiri Olsa 0 siblings, 2 replies; 7+ messages in thread From: David Laight @ 2022-10-31 11:14 UTC (permalink / raw) To: netdev@vger.kernel.org, linux-kernel@vger.kernel.org; +Cc: Jiri Olsa The 6.1-rc3 sources fail to build because bpf.h unconditionally #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) for X86_64 builds. I'm pretty sure that should depend on some other options since the compiler isn't required to support it. (The gcc 7.5.0 on my Ubunti 18.04 system certainly doesn't) The only other reference to that attribute is in the definition of 'notrace' in compiler.h. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: Linux 6.1-rc3 build fail in include/linux/bpf.h 2022-10-31 11:14 Linux 6.1-rc3 build fail in include/linux/bpf.h David Laight @ 2022-10-31 12:03 ` David Laight 2022-10-31 12:17 ` Jiri Olsa 1 sibling, 0 replies; 7+ messages in thread From: David Laight @ 2022-10-31 12:03 UTC (permalink / raw) To: David Laight, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Jiri Olsa From: David Laight > Sent: 31 October 2022 11:15 > > The 6.1-rc3 sources fail to build because bpf.h unconditionally > #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) > for X86_64 builds. > > I'm pretty sure that should depend on some other options > since the compiler isn't required to support it. > (The gcc 7.5.0 on my Ubunti 18.04 system certainly doesn't) > > The only other reference to that attribute is in the definition > of 'notrace' in compiler.h. I think patchable_function_entry was added in gcc 8. Documentation/process/changes.rst gives the minimal gcc version for building the kernel as 5.1. I doubt a increasing it to 8 is acceptable. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Linux 6.1-rc3 build fail in include/linux/bpf.h 2022-10-31 11:14 Linux 6.1-rc3 build fail in include/linux/bpf.h David Laight 2022-10-31 12:03 ` David Laight @ 2022-10-31 12:17 ` Jiri Olsa 2022-10-31 15:21 ` Peter Zijlstra 1 sibling, 1 reply; 7+ messages in thread From: Jiri Olsa @ 2022-10-31 12:17 UTC (permalink / raw) To: David Laight Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Peter Zijlstra On Mon, Oct 31, 2022 at 11:14:31AM +0000, David Laight wrote: > The 6.1-rc3 sources fail to build because bpf.h unconditionally > #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) > for X86_64 builds. > > I'm pretty sure that should depend on some other options > since the compiler isn't required to support it. > (The gcc 7.5.0 on my Ubunti 18.04 system certainly doesn't) > > The only other reference to that attribute is in the definition > of 'notrace' in compiler.h. I guess we need to make some __has_attribute check and make all that conditional cc-ing Peter jirka > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Linux 6.1-rc3 build fail in include/linux/bpf.h 2022-10-31 12:17 ` Jiri Olsa @ 2022-10-31 15:21 ` Peter Zijlstra 2022-10-31 21:53 ` Jiri Olsa 0 siblings, 1 reply; 7+ messages in thread From: Peter Zijlstra @ 2022-10-31 15:21 UTC (permalink / raw) To: Jiri Olsa Cc: David Laight, netdev@vger.kernel.org, linux-kernel@vger.kernel.org On Mon, Oct 31, 2022 at 01:17:16PM +0100, Jiri Olsa wrote: > On Mon, Oct 31, 2022 at 11:14:31AM +0000, David Laight wrote: > > The 6.1-rc3 sources fail to build because bpf.h unconditionally > > #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) > > for X86_64 builds. > > > > I'm pretty sure that should depend on some other options > > since the compiler isn't required to support it. > > (The gcc 7.5.0 on my Ubunti 18.04 system certainly doesn't) > > > > The only other reference to that attribute is in the definition > > of 'notrace' in compiler.h. > > I guess we need to make some __has_attribute check and make all that conditional > > cc-ing Peter Does something crazy like the below work? It compiles but is otherwise totally untested. --- diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 9e7d46d16032..7d7a00306d19 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -953,6 +953,10 @@ struct bpf_dispatcher { void *rw_image; u32 image_off; struct bpf_ksym ksym; +#ifdef CONFIG_HAVE_STATIC_CALL + struct static_call_key *sc_key; + void *sc_tramp; +#endif }; static __always_inline __nocfi unsigned int bpf_dispatcher_nop_func( @@ -970,6 +974,20 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key, struct bpf_attach_target_info *tgt_info); void bpf_trampoline_put(struct bpf_trampoline *tr); int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_funcs); + + +#ifdef CONFIG_HAVE_STATIC_CALL +#define BPF_DISPATCH_CALL(name) static_call(bpf_dispatcher_##name##_call)(ctx, insnsi, bpf_func) + +#define __BPF_DISPATCHER_SC_INIT(_name) \ + .sc_key = &STATIC_CALL_KEY(_name), \ + .sc_tramp = STATIC_CALL_TRAMP_ADDR(_name), + +#else +#define BPF_DISPATCH_CALL(name) bpf_func(ctx, insnsi) +#define __BPF_DISPATCHER_SC_INIT(name) +#endif + #define BPF_DISPATCHER_INIT(_name) { \ .mutex = __MUTEX_INITIALIZER(_name.mutex), \ .func = &_name##_func, \ @@ -981,32 +999,29 @@ int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_func .name = #_name, \ .lnode = LIST_HEAD_INIT(_name.ksym.lnode), \ }, \ + __BPF_DISPATCHER_SC_INIT(_name##_call) \ } -#ifdef CONFIG_X86_64 -#define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) -#else -#define BPF_DISPATCHER_ATTRIBUTES -#endif - #define DEFINE_BPF_DISPATCHER(name) \ - notrace BPF_DISPATCHER_ATTRIBUTES \ + DEFINE_STATIC_CALL(bpf_dispatcher_##name##_call, bpf_dispatcher_nop_func); \ noinline __nocfi unsigned int bpf_dispatcher_##name##_func( \ const void *ctx, \ const struct bpf_insn *insnsi, \ bpf_func_t bpf_func) \ { \ - return bpf_func(ctx, insnsi); \ + return BPF_DISPATCH_CALL(name); \ } \ EXPORT_SYMBOL(bpf_dispatcher_##name##_func); \ struct bpf_dispatcher bpf_dispatcher_##name = \ BPF_DISPATCHER_INIT(bpf_dispatcher_##name); + #define DECLARE_BPF_DISPATCHER(name) \ unsigned int bpf_dispatcher_##name##_func( \ const void *ctx, \ const struct bpf_insn *insnsi, \ bpf_func_t bpf_func); \ extern struct bpf_dispatcher bpf_dispatcher_##name; + #define BPF_DISPATCHER_FUNC(name) bpf_dispatcher_##name##_func #define BPF_DISPATCHER_PTR(name) (&bpf_dispatcher_##name) void bpf_dispatcher_change_prog(struct bpf_dispatcher *d, struct bpf_prog *from, diff --git a/kernel/bpf/dispatcher.c b/kernel/bpf/dispatcher.c index fa64b80b8bca..1ca8bd6da6bb 100644 --- a/kernel/bpf/dispatcher.c +++ b/kernel/bpf/dispatcher.c @@ -4,6 +4,7 @@ #include <linux/hash.h> #include <linux/bpf.h> #include <linux/filter.h> +#include <linux/static_call.h> /* The BPF dispatcher is a multiway branch code generator. The * dispatcher is a mechanism to avoid the performance penalty of an @@ -106,7 +107,6 @@ static void bpf_dispatcher_update(struct bpf_dispatcher *d, int prev_num_progs) { void *old, *new, *tmp; u32 noff; - int err; if (!prev_num_progs) { old = NULL; @@ -128,11 +128,10 @@ static void bpf_dispatcher_update(struct bpf_dispatcher *d, int prev_num_progs) return; } - err = bpf_arch_text_poke(d->func, BPF_MOD_JUMP, old, new); - if (err || !new) - return; + __static_call_update(d->sc_key, d->sc_tramp, new ?: &bpf_dispatcher_nop_func); - d->image_off = noff; + if (new) + d->image_off = noff; } void bpf_dispatcher_change_prog(struct bpf_dispatcher *d, struct bpf_prog *from, ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: Linux 6.1-rc3 build fail in include/linux/bpf.h 2022-10-31 15:21 ` Peter Zijlstra @ 2022-10-31 21:53 ` Jiri Olsa 2022-11-01 11:00 ` Björn Töpel 0 siblings, 1 reply; 7+ messages in thread From: Jiri Olsa @ 2022-10-31 21:53 UTC (permalink / raw) To: Peter Zijlstra, Björn Töpel, Toke Høiland-Jørgensen Cc: Jiri Olsa, David Laight, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, bpf On Mon, Oct 31, 2022 at 04:21:42PM +0100, Peter Zijlstra wrote: > On Mon, Oct 31, 2022 at 01:17:16PM +0100, Jiri Olsa wrote: > > On Mon, Oct 31, 2022 at 11:14:31AM +0000, David Laight wrote: > > > The 6.1-rc3 sources fail to build because bpf.h unconditionally > > > #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) > > > for X86_64 builds. > > > > > > I'm pretty sure that should depend on some other options > > > since the compiler isn't required to support it. > > > (The gcc 7.5.0 on my Ubunti 18.04 system certainly doesn't) > > > > > > The only other reference to that attribute is in the definition > > > of 'notrace' in compiler.h. > > > > I guess we need to make some __has_attribute check and make all that conditional > > > > cc-ing Peter > > Does something crazy like the below work? It compiles but is otherwise > totally untested. looks good it has now the ftrace nop and the jump to the dispatcher image or the bpf_dispatcher_nop_func.. great :) bpf_dispatcher_xdp_func: ffffffff81cc87b0 <load1+0xcc87b0>: ffffffff81cc87b0: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) ffffffff81cc87b5: e9 a6 fe 65 ff jmp 0xffffffff81328660 tests work for me.. Toke, Björn, could you please check? thanks, jirka > > --- > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 9e7d46d16032..7d7a00306d19 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -953,6 +953,10 @@ struct bpf_dispatcher { > void *rw_image; > u32 image_off; > struct bpf_ksym ksym; > +#ifdef CONFIG_HAVE_STATIC_CALL > + struct static_call_key *sc_key; > + void *sc_tramp; > +#endif > }; > > static __always_inline __nocfi unsigned int bpf_dispatcher_nop_func( > @@ -970,6 +974,20 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key, > struct bpf_attach_target_info *tgt_info); > void bpf_trampoline_put(struct bpf_trampoline *tr); > int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_funcs); > + > + > +#ifdef CONFIG_HAVE_STATIC_CALL > +#define BPF_DISPATCH_CALL(name) static_call(bpf_dispatcher_##name##_call)(ctx, insnsi, bpf_func) > + > +#define __BPF_DISPATCHER_SC_INIT(_name) \ > + .sc_key = &STATIC_CALL_KEY(_name), \ > + .sc_tramp = STATIC_CALL_TRAMP_ADDR(_name), > + > +#else > +#define BPF_DISPATCH_CALL(name) bpf_func(ctx, insnsi) > +#define __BPF_DISPATCHER_SC_INIT(name) > +#endif > + > #define BPF_DISPATCHER_INIT(_name) { \ > .mutex = __MUTEX_INITIALIZER(_name.mutex), \ > .func = &_name##_func, \ > @@ -981,32 +999,29 @@ int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_func > .name = #_name, \ > .lnode = LIST_HEAD_INIT(_name.ksym.lnode), \ > }, \ > + __BPF_DISPATCHER_SC_INIT(_name##_call) \ > } > > -#ifdef CONFIG_X86_64 > -#define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) > -#else > -#define BPF_DISPATCHER_ATTRIBUTES > -#endif > - > #define DEFINE_BPF_DISPATCHER(name) \ > - notrace BPF_DISPATCHER_ATTRIBUTES \ > + DEFINE_STATIC_CALL(bpf_dispatcher_##name##_call, bpf_dispatcher_nop_func); \ > noinline __nocfi unsigned int bpf_dispatcher_##name##_func( \ > const void *ctx, \ > const struct bpf_insn *insnsi, \ > bpf_func_t bpf_func) \ > { \ > - return bpf_func(ctx, insnsi); \ > + return BPF_DISPATCH_CALL(name); \ > } \ > EXPORT_SYMBOL(bpf_dispatcher_##name##_func); \ > struct bpf_dispatcher bpf_dispatcher_##name = \ > BPF_DISPATCHER_INIT(bpf_dispatcher_##name); > + > #define DECLARE_BPF_DISPATCHER(name) \ > unsigned int bpf_dispatcher_##name##_func( \ > const void *ctx, \ > const struct bpf_insn *insnsi, \ > bpf_func_t bpf_func); \ > extern struct bpf_dispatcher bpf_dispatcher_##name; > + > #define BPF_DISPATCHER_FUNC(name) bpf_dispatcher_##name##_func > #define BPF_DISPATCHER_PTR(name) (&bpf_dispatcher_##name) > void bpf_dispatcher_change_prog(struct bpf_dispatcher *d, struct bpf_prog *from, > diff --git a/kernel/bpf/dispatcher.c b/kernel/bpf/dispatcher.c > index fa64b80b8bca..1ca8bd6da6bb 100644 > --- a/kernel/bpf/dispatcher.c > +++ b/kernel/bpf/dispatcher.c > @@ -4,6 +4,7 @@ > #include <linux/hash.h> > #include <linux/bpf.h> > #include <linux/filter.h> > +#include <linux/static_call.h> > > /* The BPF dispatcher is a multiway branch code generator. The > * dispatcher is a mechanism to avoid the performance penalty of an > @@ -106,7 +107,6 @@ static void bpf_dispatcher_update(struct bpf_dispatcher *d, int prev_num_progs) > { > void *old, *new, *tmp; > u32 noff; > - int err; > > if (!prev_num_progs) { > old = NULL; > @@ -128,11 +128,10 @@ static void bpf_dispatcher_update(struct bpf_dispatcher *d, int prev_num_progs) > return; > } > > - err = bpf_arch_text_poke(d->func, BPF_MOD_JUMP, old, new); > - if (err || !new) > - return; > + __static_call_update(d->sc_key, d->sc_tramp, new ?: &bpf_dispatcher_nop_func); > > - d->image_off = noff; > + if (new) > + d->image_off = noff; > } > > void bpf_dispatcher_change_prog(struct bpf_dispatcher *d, struct bpf_prog *from, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Linux 6.1-rc3 build fail in include/linux/bpf.h 2022-10-31 21:53 ` Jiri Olsa @ 2022-11-01 11:00 ` Björn Töpel 2022-11-01 13:39 ` Björn Töpel 0 siblings, 1 reply; 7+ messages in thread From: Björn Töpel @ 2022-11-01 11:00 UTC (permalink / raw) To: Jiri Olsa, Peter Zijlstra, Toke Høiland-Jørgensen Cc: Jiri Olsa, David Laight, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, bpf Jiri Olsa <olsajiri@gmail.com> writes: > On Mon, Oct 31, 2022 at 04:21:42PM +0100, Peter Zijlstra wrote: >> >> Does something crazy like the below work? It compiles but is otherwise >> totally untested. > > looks good > > it has now the ftrace nop and the jump to the dispatcher image > or the bpf_dispatcher_nop_func.. great :) > > bpf_dispatcher_xdp_func: > > ffffffff81cc87b0 <load1+0xcc87b0>: > ffffffff81cc87b0: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) > ffffffff81cc87b5: e9 a6 fe 65 ff jmp 0xffffffff81328660 > > tests work for me.. Toke, Björn, could you please check? Awesome! Much nicer! For Peter's patch, feel free to add: Acked-by: Björn Töpel <bjorn@kernel.org> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Linux 6.1-rc3 build fail in include/linux/bpf.h 2022-11-01 11:00 ` Björn Töpel @ 2022-11-01 13:39 ` Björn Töpel 0 siblings, 0 replies; 7+ messages in thread From: Björn Töpel @ 2022-11-01 13:39 UTC (permalink / raw) To: Jiri Olsa, Peter Zijlstra, Toke Høiland-Jørgensen Cc: Jiri Olsa, David Laight, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, bpf Björn Töpel <bjorn@kernel.org> writes: > Jiri Olsa <olsajiri@gmail.com> writes: > >> tests work for me.. Toke, Björn, could you please check? > > Awesome! Much nicer! > > For Peter's patch, feel free to add: > Acked-by: Björn Töpel <bjorn@kernel.org> FWIW, I tested on commit 5aaef24b5c6d with dbe69b299884 ("bpf: Fix dispatcher patchable function entry to 5 bytes nop") reverted, and Peter's patch applied. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-01 13:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-31 11:14 Linux 6.1-rc3 build fail in include/linux/bpf.h David Laight 2022-10-31 12:03 ` David Laight 2022-10-31 12:17 ` Jiri Olsa 2022-10-31 15:21 ` Peter Zijlstra 2022-10-31 21:53 ` Jiri Olsa 2022-11-01 11:00 ` Björn Töpel 2022-11-01 13:39 ` Björn Töpel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox