On Tue, Dec 06, 2022 at 12:09:51PM -0800, Alexei Starovoitov wrote:
> On Mon, Dec 5, 2022 at 4:28 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 3bbd3f0c810c..d27b7dc77894 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2252,9 +2252,8 @@ void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
> > }
> >
> > static __always_inline
> > -void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
> > +void __bpf_trace_prog_run(struct bpf_prog *prog, u64 *args)
> > {
> > - cant_sleep();
> > if (unlikely(this_cpu_inc_return(*(prog->active)) != 1)) {
> > bpf_prog_inc_misses_counter(prog);
> > goto out;
> > @@ -2266,6 +2265,22 @@ void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
> > this_cpu_dec(*(prog->active));
> > }
> >
> > +static __always_inline
> > +void __bpf_trace_run(struct bpf_raw_event_data *data, u64 *args)
> > +{
> > + struct bpf_prog *prog = data->prog;
> > +
> > + cant_sleep();
> > + if (unlikely(!data->recursion))
> > + return __bpf_trace_prog_run(prog, args);
> > +
> > + if (unlikely(this_cpu_inc_return(*(data->recursion))))
> > + goto out;
> > + __bpf_trace_prog_run(prog, args);
> > +out:
> > + this_cpu_dec(*(data->recursion));
> > +}
>
> This is way too much run-time and memory overhead to address
> this corner case. Pls come up with some other approach.
> Sorry I don't have decent suggestions at the moment.
> For now we can simply disallow attaching to contention_begin.
>
How about this? It seems to work for me.
Thanks,
Namhyung
---
include/linux/trace_events.h | 14 +++++++
include/linux/tracepoint-defs.h | 5 +++
kernel/bpf/syscall.c | 18 ++++++++-
kernel/trace/bpf_trace.c | 65 ++++++++++++++++++++++++++++++---
4 files changed, 95 insertions(+), 7 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 20749bd9db71..461468210a77 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -742,6 +742,10 @@ void perf_event_detach_bpf_prog(struct perf_event *event);
int perf_event_query_prog_array(struct perf_event *event, void __user *info);
int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog);
int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog);
+int bpf_probe_register_norecurse(struct bpf_raw_event_map *btp, struct bpf_prog *prog,
+ struct bpf_raw_event_data *data);
+int bpf_probe_unregister_norecurse(struct bpf_raw_event_map *btp,
+ struct bpf_raw_event_data *data);
struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name);
void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp);
int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
@@ -775,6 +779,16 @@ static inline int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf
{
return -EOPNOTSUPP;
}
+static inline int bpf_probe_register_norecurse(struct bpf_raw_event_map *btp, struct bpf_prog *p,
+ struct bpf_raw_event_data *data)
+{
+ return -EOPNOTSUPP;
+}
+static inline int bpf_probe_unregister_norecurse(struct bpf_raw_event_map *btp,
+ struct bpf_raw_event_data *data)
+{
+ return -EOPNOTSUPP;
+}
static inline struct bpf_raw_event_map *bpf_get_raw_tracepoint(const char *name)
{
return NULL;
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index e7c2276be33e..e5adfe606888 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -53,6 +53,11 @@ struct bpf_raw_event_map {
u32 writable_size;
} __aligned(32);
+struct bpf_raw_event_data {
+ struct bpf_prog *prog;
+ int __percpu *active;
+};
+
/*
* If a tracepoint needs to be called from a header file, it is not
* recommended to call it directly, as tracepoints in header files
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 35972afb6850..a8be9c443306 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3144,14 +3144,24 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
struct bpf_raw_tp_link {
struct bpf_link link;
struct bpf_raw_event_map *btp;
+ struct bpf_raw_event_data data;
};
+static bool needs_recursion_check(struct bpf_raw_event_map *btp)
+{
+ return !strcmp(btp->tp->name, "contention_begin");
+}
+
static void bpf_raw_tp_link_release(struct bpf_link *link)
{
struct bpf_raw_tp_link *raw_tp =
container_of(link, struct bpf_raw_tp_link, link);
- bpf_probe_unregister(raw_tp->btp, raw_tp->link.prog);
+ if (needs_recursion_check(raw_tp->btp))
+ bpf_probe_unregister_norecurse(raw_tp->btp, &raw_tp->data);
+ else
+ bpf_probe_unregister(raw_tp->btp, raw_tp->link.prog);
+
bpf_put_raw_tracepoint(raw_tp->btp);
}
@@ -3348,7 +3358,11 @@ static int bpf_raw_tp_link_attach(struct bpf_prog *prog,
goto out_put_btp;
}
- err = bpf_probe_register(link->btp, prog);
+ if (needs_recursion_check(link->btp))
+ err = bpf_probe_register_norecurse(link->btp, prog, &link->data);
+ else
+ err = bpf_probe_register(link->btp, prog);
+
if (err) {
bpf_link_cleanup(&link_primer);
goto out_put_btp;
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 3bbd3f0c810c..edbfeff029aa 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2297,7 +2297,20 @@ void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
REPEAT(x, COPY, __DL_SEM, __SEQ_0_11); \
__bpf_trace_run(prog, args); \
} \
- EXPORT_SYMBOL_GPL(bpf_trace_run##x)
+ EXPORT_SYMBOL_GPL(bpf_trace_run##x); \
+ \
+ static void bpf_trace_run_norecurse##x(struct bpf_raw_event_data *data, \
+ REPEAT(x, SARG, __DL_COM, __SEQ_0_11)) \
+ { \
+ u64 args[x]; \
+ if (unlikely(this_cpu_inc_return(*(data->active)) != 1)) \
+ goto out; \
+ REPEAT(x, COPY, __DL_SEM, __SEQ_0_11); \
+ __bpf_trace_run(data->prog, args); \
+ out: \
+ this_cpu_dec(*(data->active)); \
+ }
+
BPF_TRACE_DEFN_x(1);
BPF_TRACE_DEFN_x(2);
BPF_TRACE_DEFN_x(3);
@@ -2311,7 +2324,23 @@ BPF_TRACE_DEFN_x(10);
BPF_TRACE_DEFN_x(11);
BPF_TRACE_DEFN_x(12);
-static int __bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
+static void *bpf_trace_norecurse_funcs[12] = {
+ (void *)bpf_trace_run_norecurse1,
+ (void *)bpf_trace_run_norecurse2,
+ (void *)bpf_trace_run_norecurse3,
+ (void *)bpf_trace_run_norecurse4,
+ (void *)bpf_trace_run_norecurse5,
+ (void *)bpf_trace_run_norecurse6,
+ (void *)bpf_trace_run_norecurse7,
+ (void *)bpf_trace_run_norecurse8,
+ (void *)bpf_trace_run_norecurse9,
+ (void *)bpf_trace_run_norecurse10,
+ (void *)bpf_trace_run_norecurse11,
+ (void *)bpf_trace_run_norecurse12,
+};
+
+static int __bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog,
+ void *func, void *data)
{
struct tracepoint *tp = btp->tp;
@@ -2325,13 +2354,12 @@ static int __bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *
if (prog->aux->max_tp_access > btp->writable_size)
return -EINVAL;
- return tracepoint_probe_register_may_exist(tp, (void *)btp->bpf_func,
- prog);
+ return tracepoint_probe_register_may_exist(tp, func, data);
}
int bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
{
- return __bpf_probe_register(btp, prog);
+ return __bpf_probe_register(btp, prog, btp->bpf_func, prog);
}
int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
@@ -2339,6 +2367,33 @@ int bpf_probe_unregister(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
return tracepoint_probe_unregister(btp->tp, (void *)btp->bpf_func, prog);
}
+int bpf_probe_register_norecurse(struct bpf_raw_event_map *btp, struct bpf_prog *prog,
+ struct bpf_raw_event_data *data)
+{
+ void *bpf_func;
+
+ data->active = alloc_percpu_gfp(int, GFP_KERNEL);
+ if (!data->active)
+ return -ENOMEM;
+
+ data->prog = prog;
+ bpf_func = bpf_trace_norecurse_funcs[btp->num_args];
+ return __bpf_probe_register(btp, prog, bpf_func, data);
+}
+
+int bpf_probe_unregister_norecurse(struct bpf_raw_event_map *btp,
+ struct bpf_raw_event_data *data)
+{
+ int err;
+ void *bpf_func;
+
+ bpf_func = bpf_trace_norecurse_funcs[btp->num_args];
+ err = tracepoint_probe_unregister(btp->tp, bpf_func, data);
+ free_percpu(data->active);
+
+ return err;
+}
+
int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
u32 *fd_type, const char **buf,
u64 *probe_offset, u64 *probe_addr)
--
2.39.0.rc0.267.gcb52ba06e7-goog
