| Message ID | 20240123092139.3698375-4-yebin10@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | support '%pd' and '%pD' for print file name | expand |
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 > >
On 2024/1/23 22:40, Masami Hiramatsu (Google) wrote: > 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. Do I need to change the other buffers on the stacks to allocate memory 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, At first I wanted to implement the extension logic in traceprobe_expand_meta_args(), but I found that the code was difficult to understand when I started writing. If fprobe_event wants to support this function, is traceprobe_expand_dentry_args() also called? Or re-encapsulate an interface to include the logic of different extensions. In this way, the same buffer is used for the entire extension process, and the extension function needs to return the information about the length of the buffer. >> + >> /* 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 I determined this value according to the extreme case that a parameter is expanded to occupy 64 bytes, and a maximum of four such large parameters are supported. >> #define MAX_STRING_SIZE PATH_MAX >> #define MAX_ARG_BUF_LEN (MAX_TRACE_ARGS * MAX_ARG_NAME_LEN) >> >> -- >> 2.31.1 >> >> >
On Wed, 24 Jan 2024 10:46:10 +0800 "yebin (H)" <yebin10@huawei.com> wrote: > > > On 2024/1/23 22:40, Masami Hiramatsu (Google) wrote: > > 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. > Do I need to change the other buffers on the stacks to allocate memory > from heap? No, that is not needed for this series, but if you want, you can :) > >> 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, > At first I wanted to implement the extension logic in > traceprobe_expand_meta_args(), > but I found that the code was difficult to understand when I started > writing. Yeah, I also found that is a bit different usage. > If fprobe_event > wants to support this function, is traceprobe_expand_dentry_args() also > called? Yes, it is for expanding '$arg*' into '$arg1 $arg2 ...' > Or re-encapsulate > an interface to include the logic of different extensions. In this way, > the same buffer is used for > the entire extension process, and the extension function needs to return > the information about > the length of the buffer. OK, I confirmed that will be too much complicated. Then can you just call it from where the traceprobe_expand_meta_args() is called, which is __trace_fprobe_create()@trace_fprobe.c ? But those should be simplified later. Thank you,
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]; 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; + /* 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 #define MAX_STRING_SIZE PATH_MAX #define MAX_ARG_BUF_LEN (MAX_TRACE_ARGS * MAX_ARG_NAME_LEN)
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(+)