| Message ID | 20240201125225.72796-2-puranjay12@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bpf, arm64: Support Exceptions | expand |
On Thu, Feb 01, 2024 at 12:52:24PM +0000, Puranjay Mohan wrote: > This will be used by bpf_throw() to unwind till the program marked as > exception boundary and run the callback with the stack of the main > program. > > This is required for supporting BPF exceptions on ARM64. > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > --- > arch/arm64/kernel/stacktrace.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > index 7f88028a00c0..66cffc5fc0be 100644 > --- a/arch/arm64/kernel/stacktrace.c > +++ b/arch/arm64/kernel/stacktrace.c > @@ -7,6 +7,7 @@ > #include <linux/kernel.h> > #include <linux/efi.h> > #include <linux/export.h> > +#include <linux/filter.h> > #include <linux/ftrace.h> > #include <linux/kprobes.h> > #include <linux/sched.h> > @@ -266,6 +267,31 @@ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, > kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs); > } > > +struct bpf_unwind_consume_entry_data { > + bool (*consume_entry)(void *cookie, u64 ip, u64 sp, u64 fp); > + void *cookie; > +}; > + > +static bool > +arch_bpf_unwind_consume_entry(const struct kunwind_state *state, void *cookie) > +{ > + struct bpf_unwind_consume_entry_data *data = cookie; > + > + return data->consume_entry(data->cookie, state->common.pc, 0, > + state->common.fp); > +} > + > +noinline noinstr void arch_bpf_stack_walk(bool (*consume_entry)(void *cookie, u64 ip, u64 sp, > + u64 fp), void *cookie) > +{ > + struct bpf_unwind_consume_entry_data data = { > + .consume_entry = consume_entry, > + .cookie = cookie, > + }; > + > + kunwind_stack_walk(arch_bpf_unwind_consume_entry, &data, current, NULL); > +} Too many "cookies", I found reading this confusing. If you ever respin, please use some different "cookie" names. I guess you want this to be merged via the bpf tree? Acked-by: Catalin Marinas <catalin.marinas@arm.com>
On Thu, Feb 22, 2024 at 11:28 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Thu, Feb 01, 2024 at 12:52:24PM +0000, Puranjay Mohan wrote: > > This will be used by bpf_throw() to unwind till the program marked as > > exception boundary and run the callback with the stack of the main > > program. > > > > This is required for supporting BPF exceptions on ARM64. > > > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > > --- > > arch/arm64/kernel/stacktrace.c | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > > index 7f88028a00c0..66cffc5fc0be 100644 > > --- a/arch/arm64/kernel/stacktrace.c > > +++ b/arch/arm64/kernel/stacktrace.c > > @@ -7,6 +7,7 @@ > > #include <linux/kernel.h> > > #include <linux/efi.h> > > #include <linux/export.h> > > +#include <linux/filter.h> > > #include <linux/ftrace.h> > > #include <linux/kprobes.h> > > #include <linux/sched.h> > > @@ -266,6 +267,31 @@ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, > > kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs); > > } > > > > +struct bpf_unwind_consume_entry_data { > > + bool (*consume_entry)(void *cookie, u64 ip, u64 sp, u64 fp); > > + void *cookie; > > +}; > > + > > +static bool > > +arch_bpf_unwind_consume_entry(const struct kunwind_state *state, void *cookie) > > +{ > > + struct bpf_unwind_consume_entry_data *data = cookie; > > + > > + return data->consume_entry(data->cookie, state->common.pc, 0, > > + state->common.fp); > > +} > > + > > +noinline noinstr void arch_bpf_stack_walk(bool (*consume_entry)(void *cookie, u64 ip, u64 sp, > > + u64 fp), void *cookie) > > +{ > > + struct bpf_unwind_consume_entry_data data = { > > + .consume_entry = consume_entry, > > + .cookie = cookie, > > + }; > > + > > + kunwind_stack_walk(arch_bpf_unwind_consume_entry, &data, current, NULL); > > +} > > Too many "cookies", I found reading this confusing. If you ever respin, > please use some different "cookie" names. > > I guess you want this to be merged via the bpf tree? We typically take bpf jit patches through bpf-next, since we do cross arch jits refactoring from time to time, but nothing like this is pending for this merge window, so if you want it to go through arm64 tree that's fine with us. > Acked-by: Catalin Marinas <catalin.marinas@arm.com> Thank you for the review!
On Thu, Feb 22, 2024 at 06:04:35PM -0800, Alexei Starovoitov wrote: > On Thu, Feb 22, 2024 at 11:28 AM Catalin Marinas > <catalin.marinas@arm.com> wrote: > > On Thu, Feb 01, 2024 at 12:52:24PM +0000, Puranjay Mohan wrote: > > > This will be used by bpf_throw() to unwind till the program marked as > > > exception boundary and run the callback with the stack of the main > > > program. > > > > > > This is required for supporting BPF exceptions on ARM64. > > > > > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > > > --- > > > arch/arm64/kernel/stacktrace.c | 26 ++++++++++++++++++++++++++ > > > 1 file changed, 26 insertions(+) [...] > > I guess you want this to be merged via the bpf tree? > > We typically take bpf jit patches through bpf-next, since > we do cross arch jits refactoring from time to time, > but nothing like this is pending for this merge window, > so if you want it to go through arm64 tree that's fine with us. I don't have any preference. I can add it on top of the other arm64 patches if there are no dependencies on it from your side.
On Tue, Feb 27, 2024 at 06:01:56PM +0000, Catalin Marinas wrote: > On Thu, Feb 22, 2024 at 06:04:35PM -0800, Alexei Starovoitov wrote: > > On Thu, Feb 22, 2024 at 11:28 AM Catalin Marinas > > <catalin.marinas@arm.com> wrote: > > > On Thu, Feb 01, 2024 at 12:52:24PM +0000, Puranjay Mohan wrote: > > > > This will be used by bpf_throw() to unwind till the program marked as > > > > exception boundary and run the callback with the stack of the main > > > > program. > > > > > > > > This is required for supporting BPF exceptions on ARM64. > > > > > > > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > > > > --- > > > > arch/arm64/kernel/stacktrace.c | 26 ++++++++++++++++++++++++++ > > > > 1 file changed, 26 insertions(+) > [...] > > > I guess you want this to be merged via the bpf tree? > > > > We typically take bpf jit patches through bpf-next, since > > we do cross arch jits refactoring from time to time, > > but nothing like this is pending for this merge window, > > so if you want it to go through arm64 tree that's fine with us. > > I don't have any preference. I can add it on top of the other arm64 > patches if there are no dependencies on it from your side. Actually, it depends on patches in bpf-next AFAICT (it doesn't apply cleanly on top of vanilla -rc3). So please take the patches through the bpf tree. Thanks.
On Tue, Feb 27, 2024 at 10:10 AM Catalin Marinas <catalin.marinas@arm.com> wrote: > > On Tue, Feb 27, 2024 at 06:01:56PM +0000, Catalin Marinas wrote: > > On Thu, Feb 22, 2024 at 06:04:35PM -0800, Alexei Starovoitov wrote: > > > On Thu, Feb 22, 2024 at 11:28 AM Catalin Marinas > > > <catalin.marinas@arm.com> wrote: > > > > On Thu, Feb 01, 2024 at 12:52:24PM +0000, Puranjay Mohan wrote: > > > > > This will be used by bpf_throw() to unwind till the program marked as > > > > > exception boundary and run the callback with the stack of the main > > > > > program. > > > > > > > > > > This is required for supporting BPF exceptions on ARM64. > > > > > > > > > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> > > > > > --- > > > > > arch/arm64/kernel/stacktrace.c | 26 ++++++++++++++++++++++++++ > > > > > 1 file changed, 26 insertions(+) > > [...] > > > > I guess you want this to be merged via the bpf tree? > > > > > > We typically take bpf jit patches through bpf-next, since > > > we do cross arch jits refactoring from time to time, > > > but nothing like this is pending for this merge window, > > > so if you want it to go through arm64 tree that's fine with us. > > > > I don't have any preference. I can add it on top of the other arm64 > > patches if there are no dependencies on it from your side. > > Actually, it depends on patches in bpf-next AFAICT (it doesn't apply > cleanly on top of vanilla -rc3). So please take the patches through the > bpf tree. Ok. Took it into bpf-next. Please take a look at these Puranjay's patchset: https://patchwork.kernel.org/project/netdevbpf/cover/20240221145106.105995-1-puranjay12@gmail.com/ It's a pretty nice performance improvement.
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index 7f88028a00c0..66cffc5fc0be 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -7,6 +7,7 @@ #include <linux/kernel.h> #include <linux/efi.h> #include <linux/export.h> +#include <linux/filter.h> #include <linux/ftrace.h> #include <linux/kprobes.h> #include <linux/sched.h> @@ -266,6 +267,31 @@ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, kunwind_stack_walk(arch_kunwind_consume_entry, &data, task, regs); } +struct bpf_unwind_consume_entry_data { + bool (*consume_entry)(void *cookie, u64 ip, u64 sp, u64 fp); + void *cookie; +}; + +static bool +arch_bpf_unwind_consume_entry(const struct kunwind_state *state, void *cookie) +{ + struct bpf_unwind_consume_entry_data *data = cookie; + + return data->consume_entry(data->cookie, state->common.pc, 0, + state->common.fp); +} + +noinline noinstr void arch_bpf_stack_walk(bool (*consume_entry)(void *cookie, u64 ip, u64 sp, + u64 fp), void *cookie) +{ + struct bpf_unwind_consume_entry_data data = { + .consume_entry = consume_entry, + .cookie = cookie, + }; + + kunwind_stack_walk(arch_bpf_unwind_consume_entry, &data, current, NULL); +} + static bool dump_backtrace_entry(void *arg, unsigned long where) { char *loglvl = arg;
This will be used by bpf_throw() to unwind till the program marked as exception boundary and run the callback with the stack of the main program. This is required for supporting BPF exceptions on ARM64. Signed-off-by: Puranjay Mohan <puranjay12@gmail.com> --- arch/arm64/kernel/stacktrace.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)