The .percent and .graph modifiers exist only for histogram values, but a
value carrying either of them has been rejected since v6.3. The example
in Documentation/trace/histogram.rst,
# echo 'hist:keys=prev_comm:vals=hitcount.percent:nohitcount' > \
events/sched/sched_switch/trigger
returns -EINVAL.
parse_field() sets the two flags only when the field is neither a key nor
a variable, that is, only on a value:
} else if (strncmp(modifier, "percent", 7) == 0) {
if (*flags & (HIST_FIELD_FL_VAR | HIST_FIELD_FL_KEY))
goto error;
*flags |= HIST_FIELD_FL_PERCENT;
__create_val_field() then rejects a value for carrying them, so no field
can reach hist_trigger_print_val(), where both are implemented.
commit e0213434fe3e ("tracing: Do not let histogram values have some
modifiers") added the check after a value with .buckets oopsed in
hist_field_name(). That happens because .buckets and .log2 make
create_hist_field() build a nested field in operands[0] which
hist_field_name() then walks into. The percent and graph flags do not
create an operand and are not read by hist_field_name(); they are only
used when printing a value.
Stop rejecting the two flags on a value. The check for variables is left
alone, where they are unreachable anyway because parse_field() rejects a
variable carrying them first.
With the two flags removed, the trigger above installs and prints as
documented:
{ prev_comm: rcu_preempt } hitcount (%): 0.00
{ prev_comm: init } hitcount (%): 99.98
Totals:
Hits: 237896
Fixes: e0213434fe3e ("tracing: Do not let histogram values have some modifiers")
Cc: [email protected]
Signed-off-by: Donggeun Yoo <[email protected]>
---
Tested under QEMU x86_64 on 1fc5a74b108f. Before the change all three of
vals=hitcount.percent
vals=hitcount.graph
vals=hitcount.percent:nohitcount
are rejected with -EINVAL; after it they install and print. A plain
'keys=prev_comm' trigger works on both, as a check that the test itself is
sound. tools/testing/selftests/ftrace trigger tests are unchanged, all 45
items identical before and after: 32 pass, 3 fail, 2 unresolved, 8
unsupported, with the failures also present on an unpatched kernel.
Nothing covers these two modifiers in selftests today, which is why the
regression stood since v6.3. I can follow up with a test case.
kernel/trace/trace_events_hist.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 963e0d6b61fd..faaff7b315f6 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -4313,8 +4313,7 @@ static int __create_val_field(struct hist_trigger_data
*hist_data,
goto err;
} else {
/* Value */
- if (hist_field->flags & (HIST_FIELD_FL_GRAPH |
HIST_FIELD_FL_PERCENT |
- HIST_FIELD_FL_BUCKET |
HIST_FIELD_FL_LOG2 |
+ if (hist_field->flags & (HIST_FIELD_FL_BUCKET |
HIST_FIELD_FL_LOG2 |
HIST_FIELD_FL_SYM |
HIST_FIELD_FL_SYM_OFFSET |
HIST_FIELD_FL_SYSCALL |
HIST_FIELD_FL_STACKTRACE))
goto err;
--
2.53.0
