On Thu, 21 Mar 2024 10:15:47 -0400
Steven Rostedt <rost...@goodmis.org> wrote:
> On Wed, 20 Mar 2024 21:29:20 +0800
> Ye Bin <yebi...@huawei.com> wrote:
>
> > Support print type '%pd' for print dentry's name.
> >
>
> The above is not a very detailed change log. A change log should state not
> only what the change is doing, but also why.
>
> Having examples of before and after would be useful in the change log.
Hm, OK. Ye, something like this.
-----
Support print type '%pd' for print dentry's name. For example "name=$arg1:%pd"
casts the `$arg1` as (struct dentry *), dereferences the "d_name.name" field
and stores it to "name" argument as a kernel string. Here is an example;
echo 'p:testprobe dput name=$arg1:%pd' > kprobe_events
echo 1 > events/kprobes/testprobe/enable
cat trace
...
sh-132 [004] ..... 333.333051: testprobe: (dput+0x4/0x20)
name="enable"
Note that this expects the given argument (e.g. $arg1) is an address of struct
dentry. User must ensure it.
-----
And add similar changelog on [2/5].
Thank you,
>
> > Signed-off-by: Ye Bin <yebi...@huawei.com>
> > ---
> > kernel/trace/trace.c | 2 +-
> > kernel/trace/trace_fprobe.c | 6 +++++
> > kernel/trace/trace_kprobe.c | 6 +++++
> > kernel/trace/trace_probe.c | 50 +++++++++++++++++++++++++++++++++++++
> > kernel/trace/trace_probe.h | 2 ++
> > 5 files changed, 65 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index b12f8384a36a..ac26b8446337 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
>
>
> > +/* @buf: *buf must be equal to NULL. Caller must to free *buf */
> > +int traceprobe_expand_dentry_args(int argc, const char *argv[], char **buf)
> > +{
> > + int i, used, ret;
> > + const int bufsize = MAX_DENTRY_ARGS_LEN;
> > + char *tmpbuf = NULL;
> > +
> > + if (*buf)
> > + return -EINVAL;
> > +
> > + used = 0;
> > + for (i = 0; i < argc; i++) {
> > + if (glob_match("*:%pd", argv[i])) {
> > + char *tmp;
> > + char *equal;
> > +
> > + if (!tmpbuf) {
> > + tmpbuf = kmalloc(bufsize, GFP_KERNEL);
> > + if (!tmpbuf)
> > + return -ENOMEM;
> > + }
> > +
> > + tmp = kstrdup(argv[i], GFP_KERNEL);
> > + if (!tmp)
> > + goto nomem;
> > +
> > + equal = strchr(tmp, '=');
> > + if (equal)
> > + *equal = '\0';
> > + tmp[strlen(argv[i]) - 4] = '\0';
> > + ret = snprintf(tmpbuf + used, bufsize - used,
> > + "%s%s+0x0(+0x%zx(%s)):string",
> > + equal ? tmp : "", equal ? "=" : "",
> > + offsetof(struct dentry, d_name.name),
> > + equal ? equal + 1 : tmp);
>
> What would be really useful is if we had a way to expose BTF here. Something
> like:
>
> "%pB:<struct>:<field>"
>
> The "%pB" would mean to look up the struct/field offsets and types via BTF,
> and create the appropriate command to find and print it.
Would you mean casing the pointer to "<struct>"?
>
> That would be much more flexible and useful as it would allow reading any
> field from any structure without having to use gdb.
>
> -- Steve
>
>
> > + kfree(tmp);
> > + if (ret >= bufsize - used)
> > + goto nomem;
> > + argv[i] = tmpbuf + used;
> > + used += ret + 1;
> > + }
> > + }
> > +
> > + *buf = tmpbuf;
> > + return 0;
> > +nomem:
> > + kfree(tmpbuf);
> > + return -ENOMEM;
> > +}
>
--
Masami Hiramatsu (Google) <mhira...@kernel.org>
