[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20230706133630.dc5b67322c840c8a2abd60a7@kernel.org>
Date: Thu, 6 Jul 2023 13:36:30 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Naveen N Rao <naveen@...nel.org>
Cc: <linux-trace-kernel@...r.kernel.org>,
Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Srikar Dronamraju <srikar@...ux.ibm.com>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] trace/kprobe: Display the actual notrace function
when rejecting a probe
Hi Naveen,
Sorry I missed this patch.
On Fri, 9 Jun 2023 13:28:09 +0530
Naveen N Rao <naveen@...nel.org> wrote:
> Trying to probe update_sd_lb_stats() using perf results in the below
> message in the kernel log:
> trace_kprobe: Could not probe notrace function _text
>
> This is because 'perf probe' specifies the kprobe location as an offset
> from '_text':
> $ sudo perf probe -D update_sd_lb_stats
> p:probe/update_sd_lb_stats _text+1830728
>
> However, the error message is misleading and doesn't help convey the
> actual notrace function that is being probed. Fix this by looking up the
> actual function name that is being probed. With this fix, we now get the
> below message in the kernel log:
> trace_kprobe: Could not probe notrace function update_sd_lb_stats.constprop.0
>
> Signed-off-by: Naveen N Rao <naveen@...nel.org>
> ---
> v2: Update within_notrace_func() stub macro with the second parameter to
> fix the build error reported by the kernel test robot.
>
> - Naveen
>
> kernel/trace/trace_kprobe.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 74adb82331dd81..2d695db5425e7c 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -449,9 +449,8 @@ static bool __within_notrace_func(unsigned long addr)
> return !ftrace_location_range(addr, addr + size - 1);
> }
>
> -static bool within_notrace_func(struct trace_kprobe *tk)
> +static bool within_notrace_func(struct trace_kprobe *tk, unsigned long addr)
> {
> - unsigned long addr = trace_kprobe_address(tk);
> char symname[KSYM_NAME_LEN], *p;
>
> if (!__within_notrace_func(addr))
> @@ -471,12 +470,14 @@ static bool within_notrace_func(struct trace_kprobe *tk)
> return true;
> }
> #else
> -#define within_notrace_func(tk) (false)
> +#define within_notrace_func(tk, addr) (false)
> #endif
Is this for avoiding redundant calling the trace_kprobe_address(tk)? If so,
please pass only 'addr' to the function since 'tk' is only used for calling
trace_kprobe_address(tk) in the within_notrace_func(). :)
Thank you,
>
> /* Internal register function - just handle k*probes and flags */
> static int __register_trace_kprobe(struct trace_kprobe *tk)
> {
> + unsigned long addr = trace_kprobe_address(tk);
> + char symname[KSYM_NAME_LEN];
> int i, ret;
>
> ret = security_locked_down(LOCKDOWN_KPROBES);
> @@ -486,9 +487,9 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
> if (trace_kprobe_is_registered(tk))
> return -EINVAL;
>
> - if (within_notrace_func(tk)) {
> + if (within_notrace_func(tk, addr)) {
> pr_warn("Could not probe notrace function %s\n",
> - trace_kprobe_symbol(tk));
> + lookup_symbol_name(addr, symname) ? trace_kprobe_symbol(tk) : symname);
> return -EINVAL;
> }
>
>
> base-commit: e46ad59233cf16daf4f3b9dd080003f01ac940fe
> --
> 2.40.1
>
--
Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists