Hi, On Mon, 2026-09-14 at 14:34 +0900, Donggeun Yoo wrote: > A hist trigger with an onmatch() action copies the key list of the > compatible histogram it finds on the matched event. Reading each key's > name straight out of key_field->field->name faults on any pseudo-field > key. 4/4 renders the key with expr_field_str() instead. The three before > it make that renderer produce something parse_field() accepts. > > Link: > https://lore.kernel.org/linux-trace-kernel/[email protected]/ > > Changes since v2: > - Rebased onto v7.3-rc4. v2 was built on 2f0c1cf72f46, before the tracing > fixes merged, and that turns out to matter -- see the next entry. > - Reinstated the stacktrace patch, now 3/4. v2 dropped it after measuring > that common_stacktrace.stacktrace parsed fine. That was true of > 2f0c1cf72f46 and is no longer true: a5e70ba87ca8 now refuses the > modifier unless the field is a real one with FILTER_STACKTRACE, so > without 3/4 a common_stacktrace key renders into a command that is > rejected. > - 1/4 is new: hist_field_print() prints the bucket size with %ld, so a > size above LONG_MAX reads back negative. Reported by sashiko-bot. > - 2/4 uses %lu for the same reason. > > Patches 1/4 through 3/4 have no effect on their own -- nothing reaches > expr_field_str() with a bucketed or stacktrace field until 4/4 renders keys > with it -- but each is needed before 4/4, and in this order no bisection > point regresses. > > The command create_field_var_hist() generates, for each kind of key the > copied histogram can carry: > > WK key unpatched patched > pid keys=pid keys=pid > pid.log2 keys=pid keys=pid.log2 > pid.buckets=10 keys=pid keys=pid.buckets=10 > common_cpu oops keys=common_cpu > common_comm oops keys=common_comm > common_timestamp oops keys=common_timestamp > common_timestamp.usecs oops keys=common_timestamp.usecs > common_stacktrace oops keys=common_stacktrace > hitcount oops keys=hitcount > > Unpatched, the .log2 and .buckets rows drop their modifier, so the > generated histogram does not bucket the way the one it mirrors does. Each > oops is a null-ptr-deref at create_field_var_hist+0x771, taken in its own > boot; the three real-field rows run the same loop to completion without > faulting, so the six are the loop reaching the faulting line rather than a > boot failure. > > And the bucket size a key can carry, read back from the trigger: > > .buckets= unpatched patched > 0 rejected rejected > -5 rejected rejected > 18446744073709551616 rejected rejected > 9223372036854775807 9223372036854775807 9223372036854775807 > 9223372036854775808 -9223372036854775808 9223372036854775808 > 18446744073709551615 -1 18446744073709551615 > > x86_64 under QEMU, CONFIG_KASAN=y, 4 CPUs, base 704340f1cd0d. A compatible > histogram on sched_waking keyed on WK, an onmatch() target on sched_switch > keyed on SK, my_synth($wakeup_lat,prio) forcing a field variable. > > Donggeun Yoo (4): > tracing: Print the bucket size as unsigned > tracing: Add the bucket size to expr_field_str() > tracing: Only report the stacktrace modifier on a real field > tracing: Fix NULL dereference when copying keys for a field variable > > kernel/trace/trace_events_hist.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) >
These look fine to me, thanks. For the set, Acked-by: Tom Zanussi <[email protected]>
