Re: [PATCH v3 26/33] tracing: Add cpu field for hist triggers

[Tom Zanussi]

Hi Steve,

On Wed, 2017-10-04 at 14:12 -0400, Steven Rostedt wrote:
> On Fri, 22 Sep 2017 15:00:06 -0500
> Tom Zanussi <tom.zanussi@xxxxxxxxxxxxxxx> wrote:
> 
> > A common key to use in a histogram is the cpuid - add a new cpu
> > 'synthetic' field for that purpose.  This field is named cpu rather
> > than $cpu or $common_cpu because 'cpu' already exists as a special
> > filter field and it makes more sense to match that rather than add
> > another name for the same thing.
> > 
> > Signed-off-by: Tom Zanussi <tom.zanussi@xxxxxxxxxxxxxxx>
> > ---
> >  Documentation/trace/events.txt   | 18 ++++++++++++++++++
> >  kernel/trace/trace_events_hist.c | 28 +++++++++++++++++++++++++++-
> >  2 files changed, 45 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/trace/events.txt b/Documentation/trace/events.txt
> > index 2cc08d4..f36fa00 100644
> > --- a/Documentation/trace/events.txt
> > +++ b/Documentation/trace/events.txt
> > @@ -668,6 +668,24 @@ The following commands are supported:
> >    The examples below provide a more concrete illustration of the
> >    concepts and typical usage patterns discussed above.
> >  
> > +  'special' event fields
> > +  ------------------------
> > +
> > +  There are a number of 'special event fields' available for use as
> > +  keys or values in a hist trigger.  These look like and behave as if
> > +  they were actual event fields, but aren't really part of the event's
> > +  field definition or format file.  They are however available for any
> > +  event, and can be used anywhere an actual event field could be.
> > +  'Special' field names are always prefixed with a '$' character to
> > +  indicate that they're not normal fields (with the exception of
> > +  'cpu', for compatibility with existing filter usage):
> > +
> > +    $common_timestamp      u64 - timestamp (from ring buffer) associated
> > +                                 with the event, in nanoseconds.  May be
> > +				 modified by .usecs to have timestamps
> > +				 interpreted as microseconds.
> > +    cpu                    int - the cpu on which the event occurred.
> > +
> >
> 
> You were going to update this too.
> 

For this one, I originally and confusingly called these 'synthetic'
event fields even though they had nothing to do with synthetic events (I
thought of them as 'synthetic' in a different sense, as not being actual
fields).  So I changed that to 'special' here to avoid the confusion.
(and I assumed your comment about moving them to the synthetic was due
to that confusion).

I took the rest of your comment as saying that these were ok together,
and in fact it does make sense to me to keep them here as part of this
patch - because we're now getting more examples of these kinds of
fields, adding a new section enumerating them starting with the second
seems to make sense here, especially considering that $common_timestamp
is mentioned elsewhere in several places in the documentation.

Tom

> -- Steve