diff options
| author | Donggeun Yoo <donggeunyoo.kernel@gmail.com> | 2026-09-17 13:59:18 +0900 |
|---|---|---|
| committer | Steven Rostedt <rostedt@goodmis.org> | 2026-10-15 20:06:53 -0400 |
| commit | d3e0dfc1ba49a0ab6aeddbb4bcde4bfc0e171a02 (patch) | |
| tree | ef25c484bb9d20c232df74e8d742a6560d39b1fd /kernel/trace | |
| parent | 443d23176812803d0fa182b8d05ef066a7b8271f (diff) | |
tracing: Print a signed histogram key as signed
A histogram sorts its rows by the key named in sort=, and for a key over a
signed event field it sorts them signed: create_tracing_map_fields() picks
the comparison function with
else
cmp_fn = tracing_map_cmp_num(field->size,
field->is_signed);
hist_trigger_print_key() does not look at is_signed at all. Every numeric
key is printed with %llu, so a sorted histogram over a signed field comes
out looking unsorted. syscalls/sys_exit_lseek carries "long ret", which
SYSCALL_FIELD() marks signed and which is negative on every error:
# echo 'hist:keys=ret:sort=ret' > events/syscalls/sys_exit_lseek/trigger
# echo 1 > events/syscalls/sys_exit_lseek/enable
# (five lseek(2) calls returning -ESPIPE, -EINVAL, -EBADF, 0 and 100)
# cat events/syscalls/sys_exit_lseek/hist
{ ret: 18446744073709551587 } hitcount: 1
{ ret: 18446744073709551594 } hitcount: 1
{ ret: 18446744073709551607 } hitcount: 1
{ ret: 0 } hitcount: 1
{ ret: 100 } hitcount: 1
The rows are in ascending order and the numbers are not, because the three
leading ones are -29, -22 and -9 rendered as their two's complement.
Take the rendering from the same place the sort order comes from: the event
field when the key has one, and the hist_field itself when it does not. The
same histogram then reads
{ ret: -29 } hitcount: 1
{ ret: -22 } hitcount: 1
{ ret: -9 } hitcount: 1
{ ret: 0 } hitcount: 1
{ ret: 100 } hitcount: 1
A ".buckets" key stays unsigned here, and not because it is right.
hist_field_bucket() divides as unsigned, so the group holding S64_MAX also
holds S64_MIN, reported at
https://lore.kernel.org/all/20260914054614.82A9B1F000FF@smtp.kernel.org/
Printing that group signed would name a range that does not contain
S64_MIN, so the grouping has to be fixed with it, in a patch of its own.
Link: https://patch.msgid.link/20260917045918.370993-1-donggeunyoo.kernel@gmail.com
Fixes: 7ef224d1d0e3 ("tracing: Add 'hist' event trigger command")
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Diffstat (limited to 'kernel/trace')
| -rw-r--r-- | kernel/trace/trace_events_hist.c | 14 |
1 files changed, 13 insertions, 1 deletions
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 7d3bb7da4dd1..5527605758cb 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -279,6 +279,14 @@ static u64 hist_field_pstring(struct hist_field *hist_field, return (u64)(unsigned long)*addr; } +static bool hist_field_is_signed(struct hist_field *hist_field) +{ + if (hist_field->field) + return hist_field->field->is_signed; + + return hist_field->is_signed; +} + static u64 hist_field_log2(struct hist_field *hist_field, struct tracing_map_elt *elt, struct trace_buffer *buffer, @@ -5574,7 +5582,11 @@ static void hist_trigger_print_key(struct seq_file *m, (char *)(key + key_field->offset)); } else { uval = *(u64 *)(key + key_field->offset); - seq_printf(m, "%s: %10llu", field_name, uval); + if (hist_field_is_signed(key_field)) + seq_printf(m, "%s: %10lld", field_name, + (s64)uval); + else + seq_printf(m, "%s: %10llu", field_name, uval); } } |
