Re: [PATCH] tracing/probes: Differentiate BTF support from start of function

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Mon, 17 Jul 2023 16:39:47 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> Having a typo in the function name to trace one of the arguments using
> BTF, gave me an error message that BTF was not supported by this kernel:
> 
> ~# echo 'p:lock __raw_spin_lock lock=lock' > /sys/kernel/tracing/kprobe_events
> -bash: echo: write error: Invalid argument
> ~# cat /sys/kernel/tracing/error_log
> [   34.382149] trace_kprobe: error: BTF is not available or not supported
>   Command: p:lock __raw_spin_lock lock=lock
>                                        ^
> 
> I added two underscores instead of one underscore for the function
> _raw_spin_lock(). But I spent too much time trying to figure out why BTF
> was not supported, because in reality it was! The problem was that the
> function wasn't found (this could happen if a function offset is used as
> well).

Thanks for reporting!

> 
> Instead, add a new error message that gives a better description of the
> problem.
> 
> ~# echo 'p:lock __raw_spin_lock lock=lock' > /sys/kernel/tracing/kprobe_events
> -bash: echo: write error: Invalid argument
> ~# cat /sys/kernel/tracing/error_log
> [   45.922742] trace_kprobe: error: BTF arg can be used only on function entry
>   Command: p:lock __raw_spin_lock lock=lock
>                                        ^

This looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!


> Fixes: b576e09701c7d ("tracing/probes: Support function parameters if BTF is available")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_probe.c | 2 +-
>  kernel/trace/trace_probe.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index cf504efd0694..328a51b1d068 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -1023,7 +1023,7 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
>  	default:
>  		if (isalpha(arg[0]) || arg[0] == '_') {	/* BTF variable */
>  			if (!tparg_is_function_entry(ctx->flags)) {
> -				trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
> +				trace_probe_log_err(ctx->offset, NOFENTRY_BTFARG);
>  				return -EINVAL;
>  			}
>  			ret = parse_btf_arg(arg, pcode, end, ctx);
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 604d6fb9c5ff..554fe2995ca1 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -493,6 +493,7 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
>  	C(BAD_ATTACH_ARG,	"Attached event does not have this field"),\
>  	C(NO_EP_FILTER,		"No filter rule after 'if'"),		\
>  	C(NOSUP_BTFARG,		"BTF is not available or not supported"),	\
> +	C(NOFENTRY_BTFARG,	"BTF arg can be used only on function entry"),	\
>  	C(NO_BTFARG,		"This variable is not found at this probe point"),\
>  	C(NO_BTF_ENTRY,		"No BTF entry for this probe point"),	\
>  	C(BAD_VAR_ARGS,		"$arg* must be an independent parameter without name etc."),\
> -- 
> 2.40.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>