Re: [PATCH v2] tracing: Fix event probe removal from dynamic events

[Masami Hiramatsu <mhiramat@kernel.org>]

On Tue, 12 Oct 2021 08:19:25 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> When an event probe is to be removed via the API to remove dynamic events,
> an -EBUSY error is returned.
> 
> This is because the removal of the event probe does not expect to see the
> event system and name that the event probe is attached to, even though
> that's part of the API to create it. As the removal of probes is to use
> the same API as they are created, fix it by first testing if the first
> parameter of the event probe to be removed matches the system and event
> that the probe is attached to, and then adjust the argc and argv of the
> parameters to match the rest of the syntax.

Hmm, this seems something wrong. Via dynamic_events interface, all the
events must be parsed equaly. If you have to pass the attached "system/event"
that's something wrong. The dynamic_events interface will accept 

-:[GROUP/]EVENT [optional arguments]

Or

!e:[GROUP/]EVENT [optional arguments]

What did you expect other that these syntax?

Thank you,

> 
> Link: https://lkml.kernel.org/r/20211011211105.48b6a5fd@oasis.local.home
> 
> Fixes: 7491e2c442781 ("tracing: Add a probe that attaches to trace events")
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> Changes since v1:
>    - amended the commit with the definition of "slash"
> 
>  kernel/trace/trace_eprobe.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> index 570d081929fb..2bcfa8da5cef 100644
> --- a/kernel/trace/trace_eprobe.c
> +++ b/kernel/trace/trace_eprobe.c
> @@ -119,6 +119,26 @@ static bool eprobe_dyn_event_match(const char *system, const char *event,
>  			int argc, const char **argv, struct dyn_event *ev)
>  {
>  	struct trace_eprobe *ep = to_trace_eprobe(ev);
> +	const char *slash;
> +
> +	/* First argument is the system/event the probe is attached to */
> +
> +	if (argc < 1)
> +		return false;
> +
> +	slash = strchr(argv[0], '/');
> +	if (!slash)
> +		slash = strchr(argv[0], '.');
> +	if (!slash)
> +		return false;
> +
> +	if (strncmp(ep->event_system, argv[0], slash - argv[0]))
> +		return false;
> +	if (strcmp(ep->event_name, slash + 1))
> +		return false;
> +
> +	argc--;
> +	argv++;
>  
>  	return strcmp(trace_probe_name(&ep->tp), event) == 0 &&
>  	    (!system || strcmp(trace_probe_group_name(&ep->tp), system) == 0) &&
> -- 
> 2.31.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>