Re: [PATCH v4 02/10] cxl: add helper to parse through all current events

[Dave Jiang <dave.jiang@intel.com>]

On 11/8/2022 3:45 PM, Verma, Vishal L wrote:
> On Tue, 2022-11-08 at 11:29 -0700, Dave Jiang wrote:
>> Add common function to iterate through and extract the events in the
>> current trace buffer. The function uses tracefs_iterate_raw_events() from
>> libtracefs to go through all the events loaded into a tep_handle. A
>> callback is provided to the API call in order to parse the event. For cxl
>> monitor, an array of interested "systems" is provided in order to filter
> 
> I couldn't tell what a 'system' means here - maybe clarify that?

It's an trace event term. In this case, the system would be "cxl". I'll 
clarify. It's trace event's category.

> 
>> for the interested events.
> 
> s/interested events/events of interest/ ?
> 
>>
>> Tested-by: Alison Schofield <alison.schofield@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>   cxl/event_trace.c |   36 ++++++++++++++++++++++++++++++++++++
>>   cxl/event_trace.h |   10 ++++++++++
>>   cxl/meson.build   |    1 +
>>   meson.build       |    2 ++
>>   4 files changed, 49 insertions(+)
>>
>> diff --git a/cxl/event_trace.c b/cxl/event_trace.c
>> index 1b1b037e48bf..4667044e4a66 100644
>> --- a/cxl/event_trace.c
>> +++ b/cxl/event_trace.c
>> @@ -16,6 +16,7 @@
>>   #include <libcxl.h>
>>   #include <uuid/uuid.h>
>>   #include <traceevent/event-parse.h>
>> +#include <tracefs/tracefs.h>
>>   #include "json.h"
>>   #include "event_trace.h"
>>   
>> @@ -199,3 +200,38 @@ err_jevent:
>>          free(jnode);
>>          return rc;
>>   }
>> +
>> +static int cxl_event_parse_cb(struct tep_event *event, struct tep_record *record,
>> +               int cpu, void *ctx)
> 
> What does the _cb mean? I couldn't tell from what it's parsing what it
> stands for - might be better to expand it?

callback. I can just drop that.

> 
>> +{
>> +       struct event_ctx *event_ctx = (struct event_ctx *)ctx;
>> +
>> +       /* Filter out all the events that the caller isn't interested in. */
>> +       if (strcmp(event->system, event_ctx->system) != 0)
>> +               return 0;
>> +
>> +       if (event_ctx->event_name) {
>> +               if (strcmp(event->name, event_ctx->event_name) != 0)
>> +                       return 0;
>> +       }
>> +
>> +       if (event_ctx->parse_event)
>> +               return event_ctx->parse_event(event, record, &event_ctx->jlist_head);
>> +
>> +       return cxl_event_to_json_callback(event, record, &event_ctx->jlist_head);
>> +}
>> +
>> +int cxl_parse_events(struct tracefs_instance *inst, struct event_ctx *ectx)
>> +{
>> +       struct tep_handle *tep;
>> +       int rc;
>> +
>> +       tep = tracefs_local_events(NULL);
>> +       if (!tep)
>> +               return -ENOMEM;
>> +
>> +       rc = tracefs_iterate_raw_events(tep, inst, NULL, 0,
>> +                       cxl_event_parse_cb, ectx);
>> +       tep_free(tep);
>> +       return rc;
>> +}
>> diff --git a/cxl/event_trace.h b/cxl/event_trace.h
>> index 00975a0b5680..582882c1eb35 100644
>> --- a/cxl/event_trace.h
>> +++ b/cxl/event_trace.h
>> @@ -11,4 +11,14 @@ struct jlist_node {
>>          struct list_node list;
>>   };
>>   
>> +struct event_ctx {
>> +       const char *system;
>> +       struct list_head jlist_head;
>> +       const char *event_name;                                 /* optional */
>> +       int (*parse_event)(struct tep_event *event, struct tep_record *record,
>> +                          struct list_head *jlist_head);       /* optional */
>> +};
>> +
>> +int cxl_parse_events(struct tracefs_instance *inst, struct event_ctx *ectx);
>> +
>>   #endif
>> diff --git a/cxl/meson.build b/cxl/meson.build
>> index 8c7733431613..c59876262e76 100644
>> --- a/cxl/meson.build
>> +++ b/cxl/meson.build
>> @@ -21,6 +21,7 @@ cxl_tool = executable('cxl',
>>       json,
>>       versiondep,
>>       traceevent,
>> +    tracefs,
>>     ],
>>     install : true,
>>     install_dir : rootbindir,
>> diff --git a/meson.build b/meson.build
>> index f611e0bdd7f3..c204c8ac52de 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -143,6 +143,8 @@ libudev = dependency('libudev')
>>   uuid = dependency('uuid')
>>   json = dependency('json-c')
>>   traceevent = dependency('libtraceevent')
>> +tracefs = dependency('libtracefs')
>> +
>>   if get_option('docs').enabled()
>>     if get_option('asciidoctor').enabled()
>>       asciidoc = find_program('asciidoctor', required : true)
>>
>>
>