* [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in boot-time tracing
@ 2021-08-02 15:30 Masami Hiramatsu
2021-08-02 15:30 ` [RFC PATCH 1/3] tracing/boot: Fix a hist trigger dependency for boot time tracing Masami Hiramatsu
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2021-08-02 15:30 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-kernel, Tom Zanussi, Masami Hiramatsu
Hi,
Here is a series of patches for boot-time tracing to add histogram
syntax extension and fixes a build issue.
Currently, the boot-time tracing only supports per-event actions
for setting trigger actions. This is enough for short actions
like 'traceon', 'traceoff', 'snapshot' etc. However, it is not good
for the 'hist' trigger action because it is usually too long to write
it in a single string especially if it has an 'onmatch' action.
Here is the new syntax.
ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist {
keys = <KEY>[,...]
values = <VAL>[,...]
sort = <SORT-KEY>[,...]
size = <ENTRIES>
name = <HISTNAME>
var { <VAR> = <EXPR> ... }
pause|continue|clear
onmax|onchange { var = <VAR>, <ACTION> [= <PARAM>] }
onmatch { event = <EVENT>, <ACTION> [= <PARAM>] }
filter = <FILTER>
}
Where <ACTION> is one of below;
trace = <EVENT>, <ARG1>[, ...]
save = <ARG1>[, ...]
snapshot
For example,
initcall.initcall_finish.actions =
"hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(initcall.initcall_start).trace(initcall_latency,func,$lat)"
This can be written as below;
initcall.initcall_finish.hist {
keys = func
var.lat = common_timestamp.usecs-$ts0
onmatch {
event = initcall.initcall_start
trace = initcall_latency, func, $lat
}
}
Also, you can add comments for each options.
TODO
====
- This version doesn't support multiple histograms on an event. For
that purpose, you still need to use per-event 'actions' option.
Maybe it can be extended to support multiple instance by adding
'_#NUM' subkey, e.g. "hist._1.keys = ...; hist._2.keys = ...".
- This also doesn't support multiple 'onmatch'/'onmax'/'onchange'
handler actions.
- Need to expand ktest testcase for checking this syntax.
Tom, this version doesn't make pause/continue/clear mutually exclusive.
And onmatch/onmax/onchange, too. As far as I can see the code, those
can be set on one histogram. But maybe I'm wrong. Please tell me if
there is some limitations or exclusiveness among those options.
Thank you,
---
Masami Hiramatsu (3):
tracing/boot: Fix a hist trigger dependency for boot time tracing
tracing/boot: Add per-event histogram action options
Documentation: tracing: Add histogram syntax to boot-time tracing
Documentation/trace/boottime-trace.rst | 81 ++++++++++++-
kernel/trace/trace_boot.c | 203 ++++++++++++++++++++++++++++++++
2 files changed, 278 insertions(+), 6 deletions(-)
--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC PATCH 1/3] tracing/boot: Fix a hist trigger dependency for boot time tracing
2021-08-02 15:30 [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in boot-time tracing Masami Hiramatsu
@ 2021-08-02 15:30 ` Masami Hiramatsu
2021-08-02 15:30 ` [RFC PATCH 2/3] tracing/boot: Add per-event histogram action options Masami Hiramatsu
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2021-08-02 15:30 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-kernel, Tom Zanussi, Masami Hiramatsu
Fixes a build error when CONFIG_HIST_TRIGGERS=n with boot-time
tracing. Since the trigger_process_regex() is defined only
when CONFIG_HIST_TRIGGERS=y, if it is disabled, the 'actions'
event option also must be disabled.
Fixes: 81a59555ff15 ("tracing/boot: Add per-event settings")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_boot.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index 94ef2d099e32..e6dc9269ad75 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -204,13 +204,14 @@ trace_boot_init_one_event(struct trace_array *tr, struct xbc_node *gnode,
else if (apply_event_filter(file, buf) < 0)
pr_err("Failed to apply filter: %s\n", buf);
}
-
+#ifdef CONFIG_HIST_TRIGGERS
xbc_node_for_each_array_value(enode, "actions", anode, p) {
if (strlcpy(buf, p, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf))
pr_err("action string is too long: %s\n", p);
else if (trigger_process_regex(file, buf) < 0)
pr_err("Failed to apply an action: %s\n", buf);
}
+#endif
if (xbc_node_find_value(enode, "enable", NULL)) {
if (trace_event_enable_disable(file, 1, 0) < 0)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH 2/3] tracing/boot: Add per-event histogram action options
2021-08-02 15:30 [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in boot-time tracing Masami Hiramatsu
2021-08-02 15:30 ` [RFC PATCH 1/3] tracing/boot: Fix a hist trigger dependency for boot time tracing Masami Hiramatsu
@ 2021-08-02 15:30 ` Masami Hiramatsu
2021-08-02 17:42 ` kernel test robot
2021-08-02 15:30 ` [RFC PATCH 3/3] Documentation: tracing: Add histogram syntax to boot-time tracing Masami Hiramatsu
` (2 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Masami Hiramatsu @ 2021-08-02 15:30 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-kernel, Tom Zanussi, Masami Hiramatsu
Add a hist-trigger action syntax support to boot-time tracing.
Currently, boot-time tracing supports per-event actions as option
strings. However, for the histogram action, it has a special syntax
and usually needs a long action definition.
To make it readable and fit to the bootconfig syntax, this introduces
a new options for histogram.
Here are the histogram action options for boot-time tracing.
ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist {
keys = <KEY>[,...]
values = <VAL>[,...]
sort = <SORT-KEY>[,...]
size = <ENTRIES>
name = <HISTNAME>
var { <VAR> = <EXPR> ... }
pause|continue|clear
onmax|onchange { var = <VAR>, <ACTION> [= <PARAM>] }
onmatch { event = <EVENT>, <ACTION> [= <PARAM>] }
filter = <FILTER>
}
Where <ACTION> is one of below;
trace = <EVENT>, <ARG1>[, ...]
save = <ARG1>[, ...]
snapshot
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
kernel/trace/trace_boot.c | 200 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 200 insertions(+)
diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index e6dc9269ad75..56e92d34a88c 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -171,6 +171,200 @@ trace_boot_add_synth_event(struct xbc_node *node, const char *event)
}
#endif
+#ifdef CONFIG_HIST_TRIGGERS
+static int __init
+append_printf(char **bufp, char *end, const char *fmt, ...)
+{
+ va_list args;
+ int ret;
+
+ if (*bufp == end)
+ return -ENOSPC;
+
+ va_start(args, fmt);
+ ret = vsnprintf(*bufp, end - *bufp, fmt, args);
+ if (ret < end - *bufp) {
+ *bufp += ret;
+ } else {
+ *bufp = end;
+ ret = -ERANGE;
+ }
+ va_end(args);
+
+ return ret;
+}
+
+static int __init
+trace_boot_hist_add_array(struct xbc_node *hnode, char **bufp,
+ char *end, const char *key)
+{
+ struct xbc_node *knode, *anode;
+ const char *p;
+
+ knode = xbc_node_find_child(hnode, key);
+ if (knode) {
+ anode = xbc_node_get_child(knode);
+ if (!anode) {
+ pr_err("hist.%s requires value(s).\n", key);
+ return -EINVAL;
+ }
+
+ append_printf(bufp, end, "%s=", key);
+ xbc_array_for_each_value(anode, p) {
+ append_printf(bufp, end, "%s,", p);
+ }
+ (*bufp)[-1] = ':';
+ } else
+ return -ENOENT;
+
+ return 0;
+}
+
+static int __init
+trace_boot_hist_add_handler(struct xbc_node *hnode, char **bufp,
+ char *end, const char *param)
+{
+ struct xbc_node *knode, *anode;
+ const char *p;
+
+ /* Compose 'handler' parameter */
+ p = xbc_node_find_value(hnode, param, NULL);
+ if (!p) {
+ pr_err("hist.%s requires '%s' option.\n",
+ xbc_node_get_data(hnode), param);
+ return -EINVAL;
+ }
+ append_printf(bufp, end, "%s(%s).", xbc_node_get_data(hnode), p);
+
+ /* Compose 'action' parameter */
+ knode = xbc_node_find_child(hnode, "trace");
+ if (!knode)
+ knode = xbc_node_find_child(hnode, "save");
+
+ if (knode) {
+ anode = xbc_node_get_child(knode);
+ if (!anode || !xbc_node_is_value(anode)) {
+ pr_err("hist.%s.%s requires value(s).\n",
+ xbc_node_get_data(hnode),
+ xbc_node_get_data(knode));
+ return -EINVAL;
+ }
+
+ append_printf(bufp, end, "%s(", xbc_node_get_data(knode));
+ xbc_array_for_each_value(anode, p) {
+ append_printf(bufp, end, "%s,", p);
+ }
+ (*bufp)[-1] = ')';
+ } else if (xbc_node_find_child(hnode, "snapshot")) {
+ append_printf(bufp, end, "snapshot()");
+ } else {
+ pr_err("hist.%s requires an action.\n",
+ xbc_node_get_data(hnode));
+ return -EINVAL;
+ }
+ append_printf(bufp, end, ":");
+
+ return 0;
+}
+
+/*
+ * Histogram boottime tracing syntax.
+ *
+ * ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist {
+ * keys = <KEY>[,...]
+ * values = <VAL>[,...]
+ * sort = <SORT-KEY>[,...]
+ * size = <ENTRIES>
+ * name = <HISTNAME>
+ * var { <VAR> = <EXPR> ... }
+ * pause|continue|clear
+ * onmax|onchange { var = <VAR>, <ACTION> [= <PARAM>] }
+ * onmatch { event = <EVENT>, <ACTION> [= <PARAM>] }
+ * filter = <FILTER>
+ * }
+ *
+ * Where <ACTION> are;
+ *
+ * trace = <EVENT>, <ARG1>[, ...]
+ * save = <ARG1>[, ...]
+ * snapshot
+ */
+static int __init
+trace_boot_compose_hist_cmd(struct xbc_node *hnode, char *buf, size_t size)
+{
+ struct xbc_node *node, *knode;
+ char *end = buf + size;
+ const char *p;
+ int ret = 0;
+
+ append_printf(&buf, end, "hist:");
+
+ ret = trace_boot_hist_add_array(hnode, &buf, end, "keys");
+ if (ret < 0) {
+ if (ret == -ENOENT)
+ pr_err("hist requires keys.\n");
+ return -EINVAL;
+ }
+
+ ret = trace_boot_hist_add_array(hnode, &buf, end, "values");
+ if (ret == -EINVAL)
+ return ret;
+ ret = trace_boot_hist_add_array(hnode, &buf, end, "sort");
+ if (ret == -EINVAL)
+ return ret;
+
+ p = xbc_node_find_value(hnode, "size", NULL);
+ if (p)
+ append_printf(&buf, end, "size=%s:", p);
+
+ p = xbc_node_find_value(hnode, "name", NULL);
+ if (p)
+ append_printf(&buf, end, "name=%s:", p);
+
+ node = xbc_node_find_child(hnode, "var");
+ if (node) {
+ xbc_node_for_each_key_value(node, knode, p) {
+ append_printf(&buf, end, "%s=%s:",
+ xbc_node_get_data(knode), p);
+ }
+ }
+
+ /* Histogram control attributes */
+ if (xbc_node_find_child(hnode, "pause"))
+ append_printf(&buf, end, "pause:");
+ if (xbc_node_find_child(hnode, "continue"))
+ append_printf(&buf, end, "continue:");
+ if (xbc_node_find_child(hnode, "clear"))
+ append_printf(&buf, end, "clear:");
+
+ /* Histogram handler and actions */
+ node = xbc_node_find_child(hnode, "onmax");
+ if (node && trace_boot_hist_add_handler(node, &buf, end, "var") < 0)
+ return -EINVAL;
+ node = xbc_node_find_child(hnode, "onchange");
+ if (node && trace_boot_hist_add_handler(node, &buf, end, "var") < 0)
+ return -EINVAL;
+ node = xbc_node_find_child(hnode, "onmatch");
+ if (node && trace_boot_hist_add_handler(node, &buf, end, "event") < 0)
+ return -EINVAL;
+
+ /* Remove the last ':' */
+ if (buf + size > end)
+ *(--buf) = '\0';
+
+ p = xbc_node_find_value(hnode, "filter", NULL);
+ if (p)
+ append_printf(&buf, end, " if %s", p);
+
+ if (buf == end) {
+ pr_err("hist exceeds the max command length.\n");
+ return -E2BIG;
+ }
+
+ return 0;
+}
+#endif
+
static void __init
trace_boot_init_one_event(struct trace_array *tr, struct xbc_node *gnode,
struct xbc_node *enode)
@@ -211,6 +405,12 @@ trace_boot_init_one_event(struct trace_array *tr, struct xbc_node *gnode,
else if (trigger_process_regex(file, buf) < 0)
pr_err("Failed to apply an action: %s\n", buf);
}
+ anode = xbc_node_find_child(enode, "hist");
+ if (anode &&
+ trace_boot_compose_hist_cmd(anode, buf, ARRAY_SIZE(buf)) == 0) {
+ if (trigger_process_regex(file, buf) < 0)
+ pr_err("Failed to apply hist trigger: %s\n", buf);
+ }
#endif
if (xbc_node_find_value(enode, "enable", NULL)) {
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH 3/3] Documentation: tracing: Add histogram syntax to boot-time tracing
2021-08-02 15:30 [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in boot-time tracing Masami Hiramatsu
2021-08-02 15:30 ` [RFC PATCH 1/3] tracing/boot: Fix a hist trigger dependency for boot time tracing Masami Hiramatsu
2021-08-02 15:30 ` [RFC PATCH 2/3] tracing/boot: Add per-event histogram action options Masami Hiramatsu
@ 2021-08-02 15:30 ` Masami Hiramatsu
2021-08-02 16:34 ` [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in " Steven Rostedt
2021-08-03 10:28 ` Masami Hiramatsu
4 siblings, 0 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2021-08-02 15:30 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-kernel, Tom Zanussi, Masami Hiramatsu
Add the documentation about histogram syntax in boot-time tracing.
This will allow user to write the histogram setting in a structured
parameters.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Documentation/trace/boottime-trace.rst | 81 ++++++++++++++++++++++++++++++--
1 file changed, 76 insertions(+), 5 deletions(-)
diff --git a/Documentation/trace/boottime-trace.rst b/Documentation/trace/boottime-trace.rst
index 8053898cfeb4..8c36785b57e9 100644
--- a/Documentation/trace/boottime-trace.rst
+++ b/Documentation/trace/boottime-trace.rst
@@ -125,6 +125,67 @@ Note that kprobe and synthetic event definitions can be written under
instance node, but those are also visible from other instances. So please
take care for event name conflict.
+Ftrace Histogram Options
+------------------------
+
+Since it is too long to write a histogram action as a string for per-event
+action option, there are tree-style options under per-event 'hist' subkey
+for the histogram actions. For the detail of the each parameter,
+please read the event histogram document [3]_.
+
+.. [3] See :ref:`Documentation/trace/histogram.rst <histogram>`
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.keys = KEY1[, KEY2[...]]
+ Set histogram key parameters. (Mandatory)
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.values = VAL1[, VAL2[...]]
+ Set histogram value parameters.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.sort = SORT1[, SORT2[...]]
+ Set histogram sort parameter options.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.size = NR_ENTRIES
+ Set histogram size (number of entries).
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.name = NAME
+ Set histogram name.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.var.VARIABLE = EXPR
+ Define a new VARIABLE by EXPR expression.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.<pause|continue|clear>
+ Set histogram control parameter.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.onmatch.event = GROUP.EVENT
+ Set histogram 'onmatch' handler matching event parameter.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.onmatch.trace = EVENT[, ARG1[...]]
+ Set histogram 'trace' action for 'onmatch'.
+ EVENT must be a synthetic event name, and ARG1... are parameters
+ for that event. Mandatory if 'onmatch.event' option is set.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.onmax.var = VAR
+ Set histogram 'onmax' handler variable parameter.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.onchange.var = VAR
+ Set histogram 'onchange' handler variable parameter.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.<onmax|onchange>.save = ARG1[, ARG2[...]]
+ Set histogram 'save' action parameters for 'onmax' or 'onchange' handler.
+ This option or below 'snapshot' option is mandatory if 'onmax.var' or
+ 'onchange.var' option is set.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.<onmax|onchange>.snapshot
+ Set histogram 'snapshot' action for 'onmax' or 'onchange' handler.
+ This option or above 'save' option is mandatory if 'onmax.var' or
+ 'onchange.var' option is set.
+
+ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist.filter = FILTER_EXPR
+ Set histogram filter expression.
+
+Note that this 'hist' option can conflict with the per-event 'actions'
+option if the 'actions' option has a histogram action.
+
When to Start
=============
@@ -159,13 +220,23 @@ below::
}
synthetic.initcall_latency {
fields = "unsigned long func", "u64 lat"
- actions = "hist:keys=func.sym,lat:vals=lat:sort=lat"
+ hist {
+ keys = func.sym, lat
+ values = lat
+ sort = lat
+ }
}
- initcall.initcall_start {
- actions = "hist:keys=func:ts0=common_timestamp.usecs"
+ initcall.initcall_start.hist {
+ keys = func
+ var.ts0 = common_timestamp.usecs
}
- initcall.initcall_finish {
- actions = "hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(initcall.initcall_start).initcall_latency(func,$lat)"
+ initcall.initcall_finish.hist {
+ keys = func
+ var.lat = common_timestamp.usecs-$ts0
+ onmatch {
+ event = initcall.initcall_start
+ trace = initcall_latency, func, $lat
+ }
}
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in boot-time tracing
2021-08-02 15:30 [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in boot-time tracing Masami Hiramatsu
` (2 preceding siblings ...)
2021-08-02 15:30 ` [RFC PATCH 3/3] Documentation: tracing: Add histogram syntax to boot-time tracing Masami Hiramatsu
@ 2021-08-02 16:34 ` Steven Rostedt
2021-08-03 10:28 ` Masami Hiramatsu
4 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2021-08-02 16:34 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: linux-kernel, Tom Zanussi
On Tue, 3 Aug 2021 00:30:10 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Hi,
>
> Here is a series of patches for boot-time tracing to add histogram
> syntax extension and fixes a build issue.
Cool, I'll try to look at this, this week.
Thanks Masami!
-- Steve
>
> Currently, the boot-time tracing only supports per-event actions
> for setting trigger actions. This is enough for short actions
> like 'traceon', 'traceoff', 'snapshot' etc. However, it is not good
> for the 'hist' trigger action because it is usually too long to write
> it in a single string especially if it has an 'onmatch' action.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 2/3] tracing/boot: Add per-event histogram action options
2021-08-02 15:30 ` [RFC PATCH 2/3] tracing/boot: Add per-event histogram action options Masami Hiramatsu
@ 2021-08-02 17:42 ` kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-08-02 17:42 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2425 bytes --]
Hi Masami,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on tip/perf/core]
[also build test WARNING on trace/for-next linux/master linus/master v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/tracing-boot-Add-histogram-syntax-support-in-boot-time-tracing/20210802-233326
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 92279a3b11a0a8486ce6b92384ddc0849eb4060f
config: x86_64-randconfig-r021-20210802 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/a10644a50f635985d898afc7413bf92579ee2250
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Masami-Hiramatsu/tracing-boot-Add-histogram-syntax-support-in-boot-time-tracing/20210802-233326
git checkout a10644a50f635985d898afc7413bf92579ee2250
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
kernel/trace/trace_boot.c: In function 'append_printf':
>> kernel/trace/trace_boot.c:185:2: warning: function 'append_printf' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
185 | ret = vsnprintf(*bufp, end - *bufp, fmt, args);
| ^~~
vim +185 kernel/trace/trace_boot.c
173
174 #ifdef CONFIG_HIST_TRIGGERS
175 static int __init
176 append_printf(char **bufp, char *end, const char *fmt, ...)
177 {
178 va_list args;
179 int ret;
180
181 if (*bufp == end)
182 return -ENOSPC;
183
184 va_start(args, fmt);
> 185 ret = vsnprintf(*bufp, end - *bufp, fmt, args);
186 if (ret < end - *bufp) {
187 *bufp += ret;
188 } else {
189 *bufp = end;
190 ret = -ERANGE;
191 }
192 va_end(args);
193
194 return ret;
195 }
196
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36696 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in boot-time tracing
2021-08-02 15:30 [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in boot-time tracing Masami Hiramatsu
` (3 preceding siblings ...)
2021-08-02 16:34 ` [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in " Steven Rostedt
@ 2021-08-03 10:28 ` Masami Hiramatsu
4 siblings, 0 replies; 7+ messages in thread
From: Masami Hiramatsu @ 2021-08-03 10:28 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: Steven Rostedt, linux-kernel, Tom Zanussi
On Tue, 3 Aug 2021 00:30:10 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Hi,
>
> Here is a series of patches for boot-time tracing to add histogram
> syntax extension and fixes a build issue.
>
> Currently, the boot-time tracing only supports per-event actions
> for setting trigger actions. This is enough for short actions
> like 'traceon', 'traceoff', 'snapshot' etc. However, it is not good
> for the 'hist' trigger action because it is usually too long to write
> it in a single string especially if it has an 'onmatch' action.
>
> Here is the new syntax.
>
> ftrace.[instance.INSTANCE.]event.GROUP.EVENT.hist {
> keys = <KEY>[,...]
> values = <VAL>[,...]
> sort = <SORT-KEY>[,...]
> size = <ENTRIES>
> name = <HISTNAME>
> var { <VAR> = <EXPR> ... }
> pause|continue|clear
> onmax|onchange { var = <VAR>, <ACTION> [= <PARAM>] }
> onmatch { event = <EVENT>, <ACTION> [= <PARAM>] }
Oops, these must use ';' as below.
onmax|onchange { var = <VAR>; <ACTION> [= <PARAM>] }
onmatch { event = <EVENT>; <ACTION> [= <PARAM>] }
> filter = <FILTER>
> }
Hmm, I found that one histogram can have several actions.
"hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(A).trace(initcall_latency,func,$lat):onmatch(B).trace(initcall_latency,func,$lat)"
This syntax can be used for the case that measures the elapsed time
from A to C, and from B to C,
event A -> event C
event B -> event C
So,
onmatch.GROUP.EVENT.<ACTION> = <PARAMS>
allows us to define multiple "onmatch" on one histogram.
onmatch.groupA.eventA.trace = synthevent, arg1, arg2
onmatch.groupB.eventB.trace = synthevent, arg1, arg2
However, there is another problem on "onmax" and "onchange".
Those will take a variable name that starts from "$", but we can not
use "$" in the subkey in bootconfig.
onmax.$lat.trace = synthevent, arg1, arg2 # this will cause error because of "$lat"
Thus, maybe we can expand the original one as
onmax|onchange[.#NUM] { var = <VAR>; <ACTION> [= <PARAM>] }
onmatch[.#NUM] { event = <EVENT>; <ACTION> [= <PARAM>] }
Then, we can add multiple handler-actions in one histogram.
onmatch.1 {
event = ...
trace = ...
}
onmatch.2 {
event = ...
trace = ...
}
Thank you,
>
> Where <ACTION> is one of below;
>
> trace = <EVENT>, <ARG1>[, ...]
> save = <ARG1>[, ...]
> snapshot
>
> For example,
>
> initcall.initcall_finish.actions =
> "hist:keys=func:lat=common_timestamp.usecs-$ts0:onmatch(initcall.initcall_start).trace(initcall_latency,func,$lat)"
>
> This can be written as below;
>
> initcall.initcall_finish.hist {
> keys = func
> var.lat = common_timestamp.usecs-$ts0
> onmatch {
> event = initcall.initcall_start
> trace = initcall_latency, func, $lat
> }
> }
>
> Also, you can add comments for each options.
>
>
> TODO
> ====
> - This version doesn't support multiple histograms on an event. For
> that purpose, you still need to use per-event 'actions' option.
> Maybe it can be extended to support multiple instance by adding
> '_#NUM' subkey, e.g. "hist._1.keys = ...; hist._2.keys = ...".
> - This also doesn't support multiple 'onmatch'/'onmax'/'onchange'
> handler actions.
> - Need to expand ktest testcase for checking this syntax.
>
> Tom, this version doesn't make pause/continue/clear mutually exclusive.
> And onmatch/onmax/onchange, too. As far as I can see the code, those
> can be set on one histogram. But maybe I'm wrong. Please tell me if
> there is some limitations or exclusiveness among those options.
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (3):
> tracing/boot: Fix a hist trigger dependency for boot time tracing
> tracing/boot: Add per-event histogram action options
> Documentation: tracing: Add histogram syntax to boot-time tracing
>
>
> Documentation/trace/boottime-trace.rst | 81 ++++++++++++-
> kernel/trace/trace_boot.c | 203 ++++++++++++++++++++++++++++++++
> 2 files changed, 278 insertions(+), 6 deletions(-)
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-03 10:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02 15:30 [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in boot-time tracing Masami Hiramatsu
2021-08-02 15:30 ` [RFC PATCH 1/3] tracing/boot: Fix a hist trigger dependency for boot time tracing Masami Hiramatsu
2021-08-02 15:30 ` [RFC PATCH 2/3] tracing/boot: Add per-event histogram action options Masami Hiramatsu
2021-08-02 17:42 ` kernel test robot
2021-08-02 15:30 ` [RFC PATCH 3/3] Documentation: tracing: Add histogram syntax to boot-time tracing Masami Hiramatsu
2021-08-02 16:34 ` [RFC PATCH 0/3] tracing/boot: Add histogram syntax support in " Steven Rostedt
2021-08-03 10:28 ` Masami Hiramatsu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.