Re: [PATCH] Documentation: trace: histogram-design: fix bracket

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 2 Sep 2026 10:17:17 -0700
Randy Dunlap <rdunlap@infradead.org> wrote:

> > diff --git a/Documentation/trace/histogram-design.rst b/Documentation/trace/histogram-design.rst
> > index 41a726cd3..b757afa22 100644
> > --- a/Documentation/trace/histogram-design.rst
> > +++ b/Documentation/trace/histogram-design.rst
> > @@ -876,7 +876,7 @@ next_pid field on this sched_switch event, we retrieve the
> >  variables specified in the wakeup_latency() trace action, and use
> >  them to generate a new wakeup_latency event into the trace stream.
> >  
> > -Note that the way the trace handlers such as wakeup_latency() (which
> > +Note that the way the trace handlers such as wakeup_latency() which
> >  could equivalently be written trace(wakeup_latency,$wakeup_lat,next_pid)
> >  are implemented, the parameters specified to the trace handler must be
> >  variables.  In this case, $wakeup_lat is obviously a variable, but  
> 
> Seems to me that the "which ..." should be a parenthetical phrase,
> i.e., with parentheses at both ends of it. IMO.
> But let's see the the TRACE maintainers have an opinion about it.

Yes, it's not an extra parenthesis but a missing one. Likely because it
would be placed in the position there is already a parenthesis.

  Note that the way the trace handlers such as wakeup_latency() (which
  could equivalently be written trace(wakeup_latency,$wakeup_lat,next_pid))  <<
  are implemented, the parameters specified to the trace handler must be
  variables.  In this case, $wakeup_lat is obviously a variable, but  

Perhaps the wording could be a bit better.

diff --git a/Documentation/trace/histogram-design.rst b/Documentation/trace/histogram-design.rst
index 41a726cd3536..ec133d0692c5 100644
--- a/Documentation/trace/histogram-design.rst
+++ b/Documentation/trace/histogram-design.rst
@@ -876,8 +876,7 @@ next_pid field on this sched_switch event, we retrieve the
 variables specified in the wakeup_latency() trace action, and use
 them to generate a new wakeup_latency event into the trace stream.
 
-Note that the way the trace handlers such as wakeup_latency() (which
-could equivalently be written trace(wakeup_latency,$wakeup_lat,next_pid)
+Note that the way the trace handlers such as wakeup_latency() 
 are implemented, the parameters specified to the trace handler must be
 variables.  In this case, $wakeup_lat is obviously a variable, but
 next_pid isn't, since it's just naming a field in the sched_switch
@@ -916,6 +915,13 @@ means it will be automatically converted into a field variable::
           onmatch(sched.sched_waking).wakeup_latency($wakeup_lat,next_pid)' >>
 	  /sys/kernel/tracing/events/sched/sched_switch/trigger
 
+Note that the above is the old way to trigger a synthetic event, whereas the
+newer way is preferred, which uses the trace() action handler::
+
+  # echo 'hist:keys=next_pid:wakeup_lat=common_timestamp.usecs-$ts0: \
+          onmatch(sched.sched_waking).trace(wakeup_latency,$wakeup_lat,next_pid)' >>
+	  /sys/kernel/tracing/events/sched/sched_switch/trigger
+
 The diagram for the sched_switch event is similar to previous examples
 but shows the additional field_vars[] array for hist_data and shows
 the linkages between the field_vars and the variables and references


-- Steve