| Message ID | 20250318180906.226841-1-douglas.raillard@arm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 21581dd4e7ff6c07d0ab577e3c32b13a74b31522 |
| Headers | show |
| Series | tracing: Ensure module defining synth event cannot be unloaded while tracing | expand |
On Tue, 18 Mar 2025 18:09:05 +0000 Douglas RAILLARD <douglas.raillard@arm.com> wrote: > From: Douglas Raillard <douglas.raillard@arm.com> > > Currently, using synth_event_delete() will fail if the event is being > used (tracing in progress), but that is normally done in the module exit > function. At that stage, failing is problematic as returning a non-zero > status means the module will become locked (impossible to unload or > reload again). > > Instead, ensure the module exit function does not get called in the > first place by increasing the module refcnt when the event is enabled. > OK, I think this is correct way to fix that scenario. Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thank you, > Signed-off-by: Douglas Raillard <douglas.raillard@arm.com> > --- > kernel/trace/trace_events_synth.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c > index e3f7d09e5512..bb7ef6e76991 100644 > --- a/kernel/trace/trace_events_synth.c > +++ b/kernel/trace/trace_events_synth.c > @@ -852,6 +852,34 @@ static struct trace_event_fields synth_event_fields_array[] = { > {} > }; > > +static int synth_event_reg(struct trace_event_call *call, > + enum trace_reg type, void *data) > +{ > + struct synth_event *event = container_of(call, struct synth_event, call); > + > + switch (type) { > + case TRACE_REG_REGISTER: > + case TRACE_REG_PERF_REGISTER: > + if (!try_module_get(event->mod)) > + return -EBUSY; > + break; > + default: > + break; > + } > + > + int ret = trace_event_reg(call, type, data); > + > + switch (type) { > + case TRACE_REG_UNREGISTER: > + case TRACE_REG_PERF_UNREGISTER: > + module_put(event->mod); > + break; > + default: > + break; > + } > + return ret; > +} > + > static int register_synth_event(struct synth_event *event) > { > struct trace_event_call *call = &event->call; > @@ -881,7 +909,7 @@ static int register_synth_event(struct synth_event *event) > goto out; > } > call->flags = TRACE_EVENT_FL_TRACEPOINT; > - call->class->reg = trace_event_reg; > + call->class->reg = synth_event_reg; > call->class->probe = trace_event_raw_event_synth; > call->data = event; > call->tp = event->tp; > -- > 2.43.0 >
diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c index e3f7d09e5512..bb7ef6e76991 100644 --- a/kernel/trace/trace_events_synth.c +++ b/kernel/trace/trace_events_synth.c @@ -852,6 +852,34 @@ static struct trace_event_fields synth_event_fields_array[] = { {} }; +static int synth_event_reg(struct trace_event_call *call, + enum trace_reg type, void *data) +{ + struct synth_event *event = container_of(call, struct synth_event, call); + + switch (type) { + case TRACE_REG_REGISTER: + case TRACE_REG_PERF_REGISTER: + if (!try_module_get(event->mod)) + return -EBUSY; + break; + default: + break; + } + + int ret = trace_event_reg(call, type, data); + + switch (type) { + case TRACE_REG_UNREGISTER: + case TRACE_REG_PERF_UNREGISTER: + module_put(event->mod); + break; + default: + break; + } + return ret; +} + static int register_synth_event(struct synth_event *event) { struct trace_event_call *call = &event->call; @@ -881,7 +909,7 @@ static int register_synth_event(struct synth_event *event) goto out; } call->flags = TRACE_EVENT_FL_TRACEPOINT; - call->class->reg = trace_event_reg; + call->class->reg = synth_event_reg; call->class->probe = trace_event_raw_event_synth; call->data = event; call->tp = event->tp;