[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cb07ac16-cc0f-7e8d-6271-cde2e02e739d@sangfor.com.cn>
Date: Mon, 12 Jun 2023 15:29:00 +0800
From: Donglin Peng <pengdonglin@...gfor.com.cn>
To: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
Cc: linux-kernel@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>,
Florent Revest <revest@...omium.org>,
Mark Rutland <mark.rutland@....com>,
Will Deacon <will@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Martin KaFai Lau <martin.lau@...ux.dev>, bpf@...r.kernel.org,
Bagas Sanjaya <bagasdotme@...il.com>,
linux-trace-kernel@...r.kernel.org,
Ding Hui <dinghui@...gfor.com.cn>, huangcun@...gfor.com.cn
Subject: Re: [PATCH v13 09/12] tracing/probes: Add BTF retval type support
On 2023/5/26 12:19, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
>
> Check the target function has non-void retval type and set the correct
> fetch type if user doesn't specify it.
> If the function returns void, $retval is rejected as below;
>
> # echo 'f unregister_kprobes%return $retval' >> dynamic_events
> sh: write error: No such file or directory
> # cat error_log
> [ 37.488397] trace_fprobe: error: This function returns 'void' type
> Command: f unregister_kprobes%return $retval
> ^
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> ---
> Changes in v8:
> - Fix wrong indentation.
> Changes in v7:
> - Introduce this as a new patch.
> ---
> kernel/trace/trace_probe.c | 69 ++++++++++++++++++++++++++++++++++++++++----
> kernel/trace/trace_probe.h | 1 +
> 2 files changed, 63 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 7318642aceb3..dfe3e1823eec 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -371,15 +371,13 @@ static const char *type_from_btf_id(struct btf *btf, s32 id)
> return NULL;
> }
>
> -static const struct btf_param *find_btf_func_param(const char *funcname, s32 *nr,
> - bool tracepoint)
> +static const struct btf_type *find_btf_func_proto(const char *funcname)
> {
> struct btf *btf = traceprobe_get_btf();
> - const struct btf_param *param;
> const struct btf_type *t;
> s32 id;
>
> - if (!btf || !funcname || !nr)
> + if (!btf || !funcname)
> return ERR_PTR(-EINVAL);
>
> id = btf_find_by_name_kind(btf, funcname, BTF_KIND_FUNC);
> @@ -396,6 +394,22 @@ static const struct btf_param *find_btf_func_param(const char *funcname, s32 *nr
> if (!btf_type_is_func_proto(t))
> return ERR_PTR(-ENOENT);
>
> + return t;
> +}
> +
> +static const struct btf_param *find_btf_func_param(const char *funcname, s32 *nr,
> + bool tracepoint)
> +{
> + const struct btf_param *param;
> + const struct btf_type *t;
> +
> + if (!funcname || !nr)
> + return ERR_PTR(-EINVAL);
> +
> + t = find_btf_func_proto(funcname);
> + if (IS_ERR(t))
> + return (const struct btf_param *)t;
> +
> *nr = btf_type_vlen(t);
> param = (const struct btf_param *)(t + 1);
>
> @@ -462,6 +476,32 @@ static const struct fetch_type *parse_btf_arg_type(int arg_idx,
> return find_fetch_type(typestr, ctx->flags);
> }
>
> +static const struct fetch_type *parse_btf_retval_type(
> + struct traceprobe_parse_context *ctx)
> +{
Can we make this a common interface so that the funcgraph-retval can
also use it to get the return type?
-- Donglin Peng
> + struct btf *btf = traceprobe_get_btf();
> + const char *typestr = NULL;
> + const struct btf_type *t;
> +
> + if (btf && ctx->funcname) {
> + t = find_btf_func_proto(ctx->funcname);
> + if (!IS_ERR(t))
> + typestr = type_from_btf_id(btf, t->type);
> + }
> +
> + return find_fetch_type(typestr, ctx->flags);
> +}
> +
> +static bool is_btf_retval_void(const char *funcname)
> +{
> + const struct btf_type *t;
> +
> + t = find_btf_func_proto(funcname);
> + if (IS_ERR(t))
> + return false;
> +
> + return t->type == 0;
> +}
> #else
> static struct btf *traceprobe_get_btf(void)
> {
> @@ -480,8 +520,15 @@ static int parse_btf_arg(const char *varname, struct fetch_insn *code,
> trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
> return -EOPNOTSUPP;
> }
> +
> #define parse_btf_arg_type(idx, ctx) \
> find_fetch_type(NULL, ctx->flags)
> +
> +#define parse_btf_retval_type(ctx) \
> + find_fetch_type(NULL, ctx->flags)
> +
> +#define is_btf_retval_void(funcname) (false)
> +
> #endif
>
> #define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))
> @@ -512,6 +559,11 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
>
> if (strcmp(arg, "retval") == 0) {
> if (ctx->flags & TPARG_FL_RETURN) {
> + if ((ctx->flags & TPARG_FL_KERNEL) &&
> + is_btf_retval_void(ctx->funcname)) {
> + err = TP_ERR_NO_RETVAL;
> + goto inval;
> + }
> code->op = FETCH_OP_RETVAL;
> return 0;
> }
> @@ -912,9 +964,12 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
> goto fail;
>
> /* Update storing type if BTF is available */
> - if (IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS) &&
> - !t && code->op == FETCH_OP_ARG)
> - parg->type = parse_btf_arg_type(code->param, ctx);
> + if (IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS) && !t) {
> + if (code->op == FETCH_OP_ARG)
> + parg->type = parse_btf_arg_type(code->param, ctx);
> + else if (code->op == FETCH_OP_RETVAL)
> + parg->type = parse_btf_retval_type(ctx);
> + }
>
> ret = -EINVAL;
> /* Store operation */
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index c864e6dea10f..eb43bea5c168 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -449,6 +449,7 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
> C(BAD_EVENT_NAME, "Event name must follow the same rules as C identifiers"), \
> C(EVENT_EXIST, "Given group/event name is already used by another event"), \
> C(RETVAL_ON_PROBE, "$retval is not available on probe"), \
> + C(NO_RETVAL, "This function returns 'void' type"), \
> C(BAD_STACK_NUM, "Invalid stack number"), \
> C(BAD_ARG_NUM, "Invalid argument number"), \
> C(BAD_VAR, "Invalid $-valiable specified"), \
>
>
>
Powered by blists - more mailing lists