Support print type '%pd' for print dentry's name.
Signed-off-by: Ye Bin <[email protected]>
---
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
@@ -5510,7 +5510,7 @@ static const char readme_msg[] =
"\t +|-[u]<offset>(<fetcharg>), \\imm-value, \\\"imm-string\"\n"
"\t type: s8/16/32/64, u8/16/32/64, x8/16/32/64, char, string, symbol,\n"
"\t b<bit-width>@<bit-offset>/<container-size>, ustring,\n"
- "\t symstr, <type>\\[<array-size>\\]\n"
+ "\t symstr, %pd, <type>\\[<array-size>\\]\n"
#ifdef CONFIG_HIST_TRIGGERS
"\t field: <stype> <name>;\n"
"\t stype: u8/u16/u32/u64, s8/s16/s32/s64, pid_t,\n"
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index 7d2ddbcfa377..988d68e906ad 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -976,6 +976,7 @@ static int __trace_fprobe_create(int argc, const char *argv[])
char gbuf[MAX_EVENT_NAME_LEN];
char sbuf[KSYM_NAME_LEN];
char abuf[MAX_BTF_ARGS_LEN];
+ char *dbuf = NULL;
bool is_tracepoint = false;
struct tracepoint *tpoint = NULL;
struct traceprobe_parse_context ctx = {
@@ -1086,6 +1087,10 @@ static int __trace_fprobe_create(int argc, const char *argv[])
argv = new_argv;
}
+ ret = traceprobe_expand_dentry_args(argc, argv, &dbuf);
+ if (ret)
+ goto out;
+
/* setup a probe */
tf = alloc_trace_fprobe(group, event, symbol, tpoint, maxactive,
argc, is_return);
@@ -1131,6 +1136,7 @@ static int __trace_fprobe_create(int argc, const char *argv[])
trace_probe_log_clear();
kfree(new_argv);
kfree(symbol);
+ kfree(dbuf);
return ret;
parse_error:
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index c4c6e0e0068b..7cbb43740b4f 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 = NULL;
struct traceprobe_parse_context ctx = { .flags = TPARG_FL_KERNEL };
switch (argv[0][0]) {
@@ -930,6 +931,10 @@ static int __trace_kprobe_create(int argc, const char *argv[])
argv = new_argv;
}
+ ret = traceprobe_expand_dentry_args(argc, argv, &dbuf);
+ if (ret)
+ goto out;
+
/* setup a probe */
tk = alloc_trace_kprobe(group, event, addr, symbol, offset, maxactive,
argc, is_return);
@@ -971,6 +976,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
trace_probe_log_clear();
kfree(new_argv);
kfree(symbol);
+ kfree(dbuf);
return ret;
parse_error:
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 34289f9c6707..a27567e16c36 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1569,6 +1569,56 @@ const char **traceprobe_expand_meta_args(int argc, const char *argv[],
return ERR_PTR(ret);
}
+/* @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);
+ kfree(tmp);
+ if (ret >= bufsize - used)
+ goto nomem;
+ argv[i] = tmpbuf + used;
+ used += ret + 1;
+ }
+ }
+
+ *buf = tmpbuf;
+ return 0;
+nomem:
+ kfree(tmpbuf);
+ return -ENOMEM;
+}
+
void traceprobe_finish_parse(struct traceprobe_parse_context *ctx)
{
clear_btf_context(ctx);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index c1877d018269..3d22fba4b63f 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)
@@ -402,6 +403,7 @@ extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
const char **traceprobe_expand_meta_args(int argc, const char *argv[],
int *new_argc, char *buf, int bufsize,
struct traceprobe_parse_context *ctx);
+extern int traceprobe_expand_dentry_args(int argc, const char *argv[], char **buf);
extern int traceprobe_update_arg(struct probe_arg *arg);
extern void traceprobe_free_probe_arg(struct probe_arg *arg);
--
2.31.1
On Wed, 20 Mar 2024 21:29:20 +0800
Ye Bin <[email protected]> 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.
> Signed-off-by: Ye Bin <[email protected]>
> ---
> 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.
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;
> +}
On Thu, 21 Mar 2024 10:15:47 -0400
Steven Rostedt <[email protected]> wrote:
> On Wed, 20 Mar 2024 21:29:20 +0800
> Ye Bin <[email protected]> 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 <[email protected]>
> > ---
> > 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) <[email protected]>
On Fri, 22 Mar 2024 00:07:59 +0900
Masami Hiramatsu (Google) <[email protected]> wrote:
> > 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>"?
BTW, for this BTF type casting, I would like to make it more naturally, like
(<struct> *)$arg1-><field> as same as other BTF args.
Thank you,
--
Masami Hiramatsu (Google) <[email protected]>
On Fri, 22 Mar 2024 00:07:59 +0900
Masami Hiramatsu (Google) <[email protected]> wrote:
> > 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>"?
I mean, instead of having:
":%pd"
We could have:
"+0(*:%pB:dentry:name):string"
Where the parsing could use BTF to see that this is a pointer to "struct dentry"
and the member field is "name".
This would also allow pretty much any other structure dereference.
That is if BTF gives structure member offsets?
-- Steve
On Fri, 22 Mar 2024 00:28:05 +0900
Masami Hiramatsu (Google) <[email protected]> wrote:
> On Fri, 22 Mar 2024 00:07:59 +0900
> Masami Hiramatsu (Google) <[email protected]> wrote:
>
> > > 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>"?
>
> BTW, for this BTF type casting, I would like to make it more naturally, like
> (<struct> *)$arg1-><field> as same as other BTF args.
>
Sure. I'm just interested in the functionality. I'll let others come up
with the syntax. ;-)
-- Steve