[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20211012233107.671725f6ec0316b3d0a9dd85@kernel.org>
Date: Tue, 12 Oct 2021 23:31:07 +0900
From: Masami Hiramatsu <mhiramat@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Tom Zanussi <zanussi@...nel.org>,
Tzvetomir Stoyanov <tz.stoyanov@...il.com>,
Yordan Karadzhov <y.karadz@...il.com>
Subject: Re: [PATCH v2] tracing: Fix event probe removal from dynamic events
On Tue, 12 Oct 2021 08:19:25 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:
> From: "Steven Rostedt (VMware)" <rostedt@...dmis.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@...dmis.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@...nel.org>
Powered by blists - more mailing lists