RE: [PATCH v2] tracing: precise log info for kretprobe addr err

[Jianlin Lv <Jianlin.Lv@arm.com>]

Hi Masami,
Thanks for your comments, it is very helpful to me.


Hi Steven,
Could you give me more detailed suggestions about the UPROBES section? I lack the knowledge of this part.
Can't fully understand your previous comments;


Thanks all for your patience.

Jianlin

> -----Original Message-----
> From: Masami Hiramatsu <mhiramat@kernel.org>
> Sent: Sunday, January 24, 2021 4:53 PM
> To: Jianlin Lv <Jianlin.Lv@arm.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>; mingo@redhat.com; linux-
> kernel@vger.kernel.org; oleg@redhat.com
> Subject: Re: [PATCH v2] tracing: precise log info for kretprobe addr err
>
> Hi Jianlin,
>
> On Sat, 23 Jan 2021 14:21:48 +0000
> Jianlin Lv <Jianlin.Lv@arm.com> wrote:
>
> > Regarding the UPROBES, I read the code of trace_uprobe_create() and
> found a place for improvement.
> >
> > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> > index 3cf7128e1ad3..8c81f04d7755 100644
> > --- a/kernel/trace/trace_uprobe.c
> > +++ b/kernel/trace/trace_uprobe.c
> > @@ -567,16 +567,18 @@ static int trace_uprobe_create(int argc, const
> char **argv)
> >         if (!filename)
> >                 return -ENOMEM;
> >
> > +       trace_probe_log_init("trace_uprobe", argc, argv);
> > +       trace_probe_log_set_index(1);   /* filename is the 2nd argument */
> > +
> >         /* Find the last occurrence, in case the path contains ':' too. */
> >         arg = strrchr(filename, ':');
> >         if (!arg || !isdigit(arg[1])) {
> > +               trace_probe_log_err(arg + 1 - filename,
> > + BAD_UPROBE_OFFS);
> >                 kfree(filename);
> > -               return -ECANCELED;
> > +               ret = -EINVAL;
> > +               goto out;
>
> Sorry, it's not a bug, but an expected behavior, because this is checking
> routine which identify the passed command is mine (uprobe event definition)
> or others (e.g. kprobe event definition).
>
> Note that the dyn_event_operations::create operator will be used from
> dyn_event, which passes the command string from
> TRACEFS/dynamic_events to each create operator and check the return is -
> ECANCELED.
> The -ECANCELED is not an error, it means "it is not mine, but maybe others."
> See create_dyn_event(int argc, char **argv) in kernel/trace/trace_dynevent.c.
>
> Thank you,
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.