Received-SPF: pass (google.com: domain of linux-kernel+bounces-35463-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 23 Jan 2024 23:40:13 +0900
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Ye Bin <yebin10@huawei.com>
Cc: <rostedt@goodmis.org>, <mathieu.desnoyers@efficios.com>,
 <linux-trace-kernel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 3/7] tracing/probes: support '%pd' type for print
 struct dentry's name
Message-Id: <20240123234013.52b1834908da7be0272e7a73@kernel.org>
In-Reply-To: <20240123092139.3698375-4-yebin10@huawei.com>
References: <20240123092139.3698375-1-yebin10@huawei.com>
	<20240123092139.3698375-4-yebin10@huawei.com>
Precedence: bulk
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Tue, 23 Jan 2024 17:21:35 +0800
Ye Bin <yebin10@huawei.com> wrote:

> Similar to '%pd' for printk, use '%pd' for print struct dentry's name.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  kernel/trace/trace_kprobe.c | 6 ++++++
>  kernel/trace/trace_probe.h  | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index c4c6e0e0068b..00b74530fbad 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -779,6 +779,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>  	char buf[MAX_EVENT_NAME_LEN];
>  	char gbuf[MAX_EVENT_NAME_LEN];
>  	char abuf[MAX_BTF_ARGS_LEN];
> +	char dbuf[MAX_DENTRY_ARGS_LEN];

Hmm, no, I don't like to expand stack anymore. Please allocate it
from heap.

>  	struct traceprobe_parse_context ctx = { .flags = TPARG_FL_KERNEL };
>  
>  	switch (argv[0][0]) {
> @@ -930,6 +931,11 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>  		argv = new_argv;
>  	}
>  
> +	ret = traceprobe_expand_dentry_args(argc, argv, dbuf,
> +					    MAX_DENTRY_ARGS_LEN);
> +	if (ret)
> +		goto out;

And calling this here will not cover the trace_fprobe. 

Could you call this from traceprobe_expand_meta_args() instead of
calling it directly from trace_kprobe? Then it can be used from
fprobe_event too.

Thank you,

> +
>  	/* setup a probe */
>  	tk = alloc_trace_kprobe(group, event, addr, symbol, offset, maxactive,
>  				argc, is_return);
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 553371a4e0b1..d9c053824975 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -34,6 +34,7 @@
>  #define MAX_ARRAY_LEN		64
>  #define MAX_ARG_NAME_LEN	32
>  #define MAX_BTF_ARGS_LEN	128
> +#define MAX_DENTRY_ARGS_LEN	256

Why do you think 

>  #define MAX_STRING_SIZE		PATH_MAX
>  #define MAX_ARG_BUF_LEN		(MAX_TRACE_ARGS * MAX_ARG_NAME_LEN)
>  
> -- 
> 2.31.1
> 
> 


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