[PATCH 0/7] ndctl: cxl: add monitor support for trace events

[PATCH 0/7] ndctl: cxl: add monitor support for trace events

[Dave Jiang]

This patch series for ndctl implements the monitor command for the cxl tool.
The initial implementation will collect CXL trace events emitted by the
kernel. libtraceevent and libtracefs will be used to parse the trace
event buffer. The monitor will pend on an epoll fd and wait for new event
entries to be posted. The output will be in json format. By default the events
are emitted to stdio, but can also be logged to a file. Each event is converted
to a JSON object and logged as such. All the fields exported are read by the
monitor code and added to the JSON object.

---

Dave Jiang (7):
      ndctl: cxl: add helper function to parse trace event to json object
      ndctl: cxl: add helper to parse through all current events
      ndctl: cxl: add common function to enable event trace
      ndctl: cxl: add common function to disable event trace
      ndctl: cxl: add monitor function for event trace events
      ndctl: cxl: add logging functions for monitor
      ndctl: cxl: add monitor command to cxl


 cxl/builtin.h     |   1 +
 cxl/cxl.c         |   1 +
 cxl/event_trace.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++
 cxl/event_trace.h |  23 +++++
 cxl/meson.build   |   4 +
 cxl/monitor.c     | 235 +++++++++++++++++++++++++++++++++++++++++++++
 meson.build       |   3 +
 7 files changed, 503 insertions(+)
 create mode 100644 cxl/event_trace.c
 create mode 100644 cxl/event_trace.h
 create mode 100644 cxl/monitor.c

--

[PATCH 1/7] ndctl: cxl: add helper function to parse trace event to json object

[Dave Jiang]

Add the helper function that parses a trace event captured by
libtraceevent in a tep handle. All the parsed fields are added to a json
object. The json object is added to the provided list in the input parameter.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/event_trace.c |  166 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 cxl/event_trace.h |   14 ++++
 cxl/meson.build   |    2 +
 meson.build       |    1 
 4 files changed, 183 insertions(+)
 create mode 100644 cxl/event_trace.c
 create mode 100644 cxl/event_trace.h

diff --git a/cxl/event_trace.c b/cxl/event_trace.c
new file mode 100644
index 000000000000..ffa2a9b9b036
--- /dev/null
+++ b/cxl/event_trace.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2022, Intel Corp. All rights reserved.
+#include <stdio.h>
+#include <json-c/json.h>
+#include <util/json.h>
+#include <util/util.h>
+#include <util/parse-options.h>
+#include <util/parse-configs.h>
+#include <util/strbuf.h>
+#include <util/sysfs.h>
+#include <ccan/list/list.h>
+#include <ndctl/ndctl.h>
+#include <ndctl/libndctl.h>
+#include <sys/epoll.h>
+#include <sys/stat.h>
+#include <libcxl.h>
+#include <uuid/uuid.h>
+#include <traceevent/event-parse.h>
+#include "json.h"
+#include "event_trace.h"
+
+#define _GNU_SOURCE
+#include <string.h>
+
+static struct json_object *num_to_json(void *num, int size)
+{
+	if (size <= 4)
+		return json_object_new_int(*(int *)num);
+
+	return util_json_object_hex(*(unsigned long long *)num, 0);
+}
+
+static int cxl_event_to_json_callback(struct tep_event *event,
+		struct tep_record *record, struct list_head *jlist_head)
+{
+	struct tep_format_field **fields;
+	struct json_object *jevent, *jobj, *jarray;
+	struct jlist_node *jnode;
+	int i, j, rc = 0;
+
+	jnode = malloc(sizeof(*jnode));
+	if (!jnode)
+		return -ENOMEM;
+
+	jevent = json_object_new_object();
+	if (!jevent) {
+		rc = -ENOMEM;
+		goto err_jevent;
+	}
+	jnode->jobj = jevent;
+
+	fields = tep_event_fields(event);
+	if (!fields) {
+		rc = -ENOENT;
+		goto err;
+	}
+
+	jobj = json_object_new_string(event->system);
+	if (!jobj) {
+		rc = -ENOMEM;
+		goto err;
+	}
+	json_object_object_add(jevent, "system", jobj);
+
+	jobj = json_object_new_string(event->name);
+	if (!jobj) {
+		rc = -ENOMEM;
+		goto err;
+	}
+	json_object_object_add(jevent, "event", jobj);
+
+	jobj = json_object_new_uint64(record->ts);
+	if (!jobj) {
+		rc = -ENOMEM;
+		goto err;
+	}
+	json_object_object_add(jevent, "timestamp", jobj);
+
+	for (i = 0; fields[i]; i++) {
+		struct tep_format_field *f = fields[i];
+		int len;
+		char *tmp;
+
+		tmp = strcasestr(f->type, "char[]");
+		if (tmp) { /* event field is a string */
+			char *str;
+
+			str = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
+			if (!str)
+				continue;
+
+			jobj = json_object_new_string(str);
+			if (!jobj) {
+				rc = -ENOMEM;
+				goto err;
+			}
+
+			json_object_object_add(jevent, f->name, jobj);
+		} else if (f->arraylen) { /* data array */
+			unsigned char *data;
+			int chunks;
+
+			data = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
+			if (!data)
+				continue;
+
+			/* check to see if we have a UUID */
+			tmp = strcasestr(f->name, "uuid");
+			if (tmp && f->arraylen == 16) {
+				char uuid[SYSFS_ATTR_SIZE];
+
+				uuid_unparse(data, uuid);
+				jobj = json_object_new_string(uuid);
+				if (!jobj) {
+					rc = -ENOMEM;
+					goto err;
+				}
+
+				json_object_object_add(jevent, f->name, jobj);
+				continue;
+			}
+
+			jarray = json_object_new_array();
+			if (!jarray) {
+				rc = -ENOMEM;
+				goto err;
+			}
+
+			chunks = f->size / f->elementsize;
+			for (j = 0; j < chunks; j++) {
+				jobj = num_to_json(data, f->elementsize);
+				if (!jobj) {
+					json_object_put(jarray);
+					return -ENOMEM;
+				}
+				json_object_array_add(jarray, jobj);
+				data += f->elementsize;
+			}
+
+			json_object_object_add(jevent, f->name, jarray);
+		} else { /* single number */
+			char *data;
+
+			data = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
+			if (!data)
+				continue;
+
+			jobj = num_to_json(data, f->elementsize);
+			if (!jobj) {
+				rc = -ENOMEM;
+				goto err;
+			}
+
+			json_object_object_add(jevent, f->name, jobj);
+		}
+	}
+
+	list_add_tail(jlist_head, &jnode->list);
+	return 0;
+
+err:
+	json_object_put(jevent);
+err_jevent:
+	free(jnode);
+	return rc;
+}
diff --git a/cxl/event_trace.h b/cxl/event_trace.h
new file mode 100644
index 000000000000..00975a0b5680
--- /dev/null
+++ b/cxl/event_trace.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2022 Intel Corporation. All rights reserved. */
+#ifndef __CXL_EVENT_TRACE_H__
+#define __CXL_EVENT_TRACE_H__
+
+#include <json-c/json.h>
+#include <ccan/list/list.h>
+
+struct jlist_node {
+	struct json_object *jobj;
+	struct list_node list;
+};
+
+#endif
diff --git a/cxl/meson.build b/cxl/meson.build
index f2474aaa6e2e..8c7733431613 100644
--- a/cxl/meson.build
+++ b/cxl/meson.build
@@ -7,6 +7,7 @@ cxl_src = [
   'memdev.c',
   'json.c',
   'filter.c',
+  'event_trace.c',
 ]
 
 cxl_tool = executable('cxl',
@@ -19,6 +20,7 @@ cxl_tool = executable('cxl',
     kmod,
     json,
     versiondep,
+    traceevent,
   ],
   install : true,
   install_dir : rootbindir,
diff --git a/meson.build b/meson.build
index 20a646d135c7..f611e0bdd7f3 100644
--- a/meson.build
+++ b/meson.build
@@ -142,6 +142,7 @@ kmod = dependency('libkmod')
 libudev = dependency('libudev')
 uuid = dependency('uuid')
 json = dependency('json-c')
+traceevent = dependency('libtraceevent')
 if get_option('docs').enabled()
   if get_option('asciidoctor').enabled()
     asciidoc = find_program('asciidoctor', required : true)

Re: [PATCH 1/7] ndctl: cxl: add helper function to parse trace event to json object

[Alison Schofield]

On Wed, Sep 14, 2022 at 01:47:55PM -0700, Dave Jiang wrote:
> Add the helper function that parses a trace event captured by
> libtraceevent in a tep handle. All the parsed fields are added to a json
> object. The json object is added to the provided list in the input parameter.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Hi Dave,

I've been using these parsing patches, and it all works for the purpose 
of parsing the cxl device poison list.

Tested-by: Alison Schofield <alison.schofield@intel.com>

One comment below...

-snip-

> ---
> +
> +	for (i = 0; fields[i]; i++) {
> +		struct tep_format_field *f = fields[i];
> +		int len;
> +		char *tmp;
> +
> +		tmp = strcasestr(f->type, "char[]");
> +		if (tmp) { /* event field is a string */
> +			char *str;
> +
> +			str = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
> +			if (!str)
> +				continue;
> +
> +			jobj = json_object_new_string(str);
> +			if (!jobj) {
> +				rc = -ENOMEM;
> +				goto err;
> +			}
> +
> +			json_object_object_add(jevent, f->name, jobj);
> +		} else if (f->arraylen) { /* data array */
> +			unsigned char *data;
> +			int chunks;
> +
> +			data = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
> +			if (!data)
> +				continue;
> +
> +			/* check to see if we have a UUID */
> +			tmp = strcasestr(f->name, "uuid");
> +			if (tmp && f->arraylen == 16) {
> +				char uuid[SYSFS_ATTR_SIZE];
> +
> +				uuid_unparse(data, uuid);
> +				jobj = json_object_new_string(uuid);
> +				if (!jobj) {
> +					rc = -ENOMEM;
> +					goto err;
> +				}
> +
> +				json_object_object_add(jevent, f->name, jobj);
> +				continue;
> +			}

Insted of comparing the field name, is it possible to check for
f->type of 'uuid_t'?


> +
> +			jarray = json_object_new_array();
> +			if (!jarray) {
> +				rc = -ENOMEM;
> +				goto err;
> +			}
> +
> +			chunks = f->size / f->elementsize;
> +			for (j = 0; j < chunks; j++) {
> +				jobj = num_to_json(data, f->elementsize);
> +				if (!jobj) {
> +					json_object_put(jarray);
> +					return -ENOMEM;
> +				}
> +				json_object_array_add(jarray, jobj);
> +				data += f->elementsize;
> +			}
> +
> +			json_object_object_add(jevent, f->name, jarray);
> +		} else { /* single number */
> +			char *data;
> +
> +			data = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
> +			if (!data)
> +				continue;
> +
> +			jobj = num_to_json(data, f->elementsize);
> +			if (!jobj) {
> +				rc = -ENOMEM;
> +				goto err;
> +			}
> +
> +			json_object_object_add(jevent, f->name, jobj);
> +		}
> +	}
> +
> +	list_add_tail(jlist_head, &jnode->list);
> +	return 0;
> +
> +err:
> +	json_object_put(jevent);
> +err_jevent:
> +	free(jnode);
> +	return rc;
> +}

snip

Re: [PATCH 1/7] ndctl: cxl: add helper function to parse trace event to json object

[Dave Jiang]

On 10/31/2022 1:01 PM, Alison Schofield wrote:
> On Wed, Sep 14, 2022 at 01:47:55PM -0700, Dave Jiang wrote:
>> Add the helper function that parses a trace event captured by
>> libtraceevent in a tep handle. All the parsed fields are added to a json
>> object. The json object is added to the provided list in the input parameter.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> Hi Dave,
> 
> I've been using these parsing patches, and it all works for the purpose
> of parsing the cxl device poison list.
> 
> Tested-by: Alison Schofield <alison.schofield@intel.com>

Thanks!

> 
> One comment below...
> 
> -snip-
> 
>> ---
>> +
>> +	for (i = 0; fields[i]; i++) {
>> +		struct tep_format_field *f = fields[i];
>> +		int len;
>> +		char *tmp;
>> +
>> +		tmp = strcasestr(f->type, "char[]");
>> +		if (tmp) { /* event field is a string */
>> +			char *str;
>> +
>> +			str = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
>> +			if (!str)
>> +				continue;
>> +
>> +			jobj = json_object_new_string(str);
>> +			if (!jobj) {
>> +				rc = -ENOMEM;
>> +				goto err;
>> +			}
>> +
>> +			json_object_object_add(jevent, f->name, jobj);
>> +		} else if (f->arraylen) { /* data array */
>> +			unsigned char *data;
>> +			int chunks;
>> +
>> +			data = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
>> +			if (!data)
>> +				continue;
>> +
>> +			/* check to see if we have a UUID */
>> +			tmp = strcasestr(f->name, "uuid");
>> +			if (tmp && f->arraylen == 16) {
>> +				char uuid[SYSFS_ATTR_SIZE];
>> +
>> +				uuid_unparse(data, uuid);
>> +				jobj = json_object_new_string(uuid);
>> +				if (!jobj) {
>> +					rc = -ENOMEM;
>> +					goto err;
>> +				}
>> +
>> +				json_object_object_add(jevent, f->name, jobj);
>> +				continue;
>> +			}
> 
> Insted of comparing the field name, is it possible to check for
> f->type of 'uuid_t'?
> 

Yes. I talked to Ira and he'll change his uuid to uuid_t from u8[]. That 
should make it better. Thanks for pointing that out.

> 
>> +
>> +			jarray = json_object_new_array();
>> +			if (!jarray) {
>> +				rc = -ENOMEM;
>> +				goto err;
>> +			}
>> +
>> +			chunks = f->size / f->elementsize;
>> +			for (j = 0; j < chunks; j++) {
>> +				jobj = num_to_json(data, f->elementsize);
>> +				if (!jobj) {
>> +					json_object_put(jarray);
>> +					return -ENOMEM;
>> +				}
>> +				json_object_array_add(jarray, jobj);
>> +				data += f->elementsize;
>> +			}
>> +
>> +			json_object_object_add(jevent, f->name, jarray);
>> +		} else { /* single number */
>> +			char *data;
>> +
>> +			data = tep_get_field_raw(NULL, event, f->name, record, &len, 0);
>> +			if (!data)
>> +				continue;
>> +
>> +			jobj = num_to_json(data, f->elementsize);
>> +			if (!jobj) {
>> +				rc = -ENOMEM;
>> +				goto err;
>> +			}
>> +
>> +			json_object_object_add(jevent, f->name, jobj);
>> +		}
>> +	}
>> +
>> +	list_add_tail(jlist_head, &jnode->list);
>> +	return 0;
>> +
>> +err:
>> +	json_object_put(jevent);
>> +err_jevent:
>> +	free(jnode);
>> +	return rc;
>> +}
> 
> snip
> 

Re: [PATCH 1/7] ndctl: cxl: add helper function to parse trace event to json object

[Ira Weiny]

On Mon, Oct 31, 2022 at 02:37:02PM -0700, Jiang, Dave wrote:
> 
> 
> On 10/31/2022 1:01 PM, Alison Schofield wrote:

[snip]

> > > +			/* check to see if we have a UUID */
> > > +			tmp = strcasestr(f->name, "uuid");
> > > +			if (tmp && f->arraylen == 16) {
> > > +				char uuid[SYSFS_ATTR_SIZE];
> > > +
> > > +				uuid_unparse(data, uuid);
> > > +				jobj = json_object_new_string(uuid);
> > > +				if (!jobj) {
> > > +					rc = -ENOMEM;
> > > +					goto err;
> > > +				}
> > > +
> > > +				json_object_object_add(jevent, f->name, jobj);
> > > +				continue;
> > > +			}
> > 
> > Insted of comparing the field name, is it possible to check for
> > f->type of 'uuid_t'?
> > 
> 
> Yes. I talked to Ira and he'll change his uuid to uuid_t from u8[]. That
> should make it better. Thanks for pointing that out.

So after unborking my qemu I was able to actually try this.  And it did not
work the way I thought it would.

However, in the end Dave and I figured out how to make it work using the
__field_struct() TP entry type in the kernel and some mods to cxl.

Thanks Alison this is much better.

Ira

[PATCH 2/7] ndctl: cxl: add helper to parse through all current events

[Dave Jiang]

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
for the interested events.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/event_trace.c |   35 +++++++++++++++++++++++++++++++++++
 cxl/event_trace.h |    7 +++++++
 cxl/meson.build   |    1 +
 meson.build       |    2 ++
 4 files changed, 45 insertions(+)

diff --git a/cxl/event_trace.c b/cxl/event_trace.c
index ffa2a9b9b036..d5e67b55a4d8 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"
 
@@ -164,3 +165,37 @@ err_jevent:
 	free(jnode);
 	return rc;
 }
+
+static int cxl_event_parse_cb(struct tep_event *event, struct tep_record *record,
+		int cpu, void *ctx)
+{
+	struct event_ctx *event_ctx = (struct event_ctx *)ctx;
+	int rc, i;
+
+	/* Filter out all the events that the caller isn't interested in. */
+	for (i = 0; event_ctx->systems[i]; i++) {
+		if (strcmp(event->system, event_ctx->systems[i]) != 0)
+			continue;
+
+		rc = cxl_event_to_json_callback(event, record, &event_ctx->jlist_head);
+		if (rc < 0)
+			return rc;
+	}
+
+	return 0;
+}
+
+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..98cfbb4602e1 100644
--- a/cxl/event_trace.h
+++ b/cxl/event_trace.h
@@ -11,4 +11,11 @@ struct jlist_node {
 	struct list_node list;
 };
 
+struct event_ctx {
+	const char **systems;
+	struct list_head jlist_head;
+};
+
+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)

Re: [PATCH 2/7] ndctl: cxl: add helper to parse through all current events

[Alison Schofield]

On Wed, Sep 14, 2022 at 01:48:01PM -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
> for the interested events.

Also been testing this one, and WFM. I'll look for the addition of the
caller defined parsing callback in the next rev.

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  cxl/event_trace.c |   35 +++++++++++++++++++++++++++++++++++
>  cxl/event_trace.h |    7 +++++++
>  cxl/meson.build   |    1 +
>  meson.build       |    2 ++
>  4 files changed, 45 insertions(+)
> 
> diff --git a/cxl/event_trace.c b/cxl/event_trace.c
> index ffa2a9b9b036..d5e67b55a4d8 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"
>  
> @@ -164,3 +165,37 @@ err_jevent:
>  	free(jnode);
>  	return rc;
>  }
> +
> +static int cxl_event_parse_cb(struct tep_event *event, struct tep_record *record,
> +		int cpu, void *ctx)
> +{
> +	struct event_ctx *event_ctx = (struct event_ctx *)ctx;
> +	int rc, i;
> +
> +	/* Filter out all the events that the caller isn't interested in. */
> +	for (i = 0; event_ctx->systems[i]; i++) {
> +		if (strcmp(event->system, event_ctx->systems[i]) != 0)
> +			continue;
> +
> +		rc = cxl_event_to_json_callback(event, record, &event_ctx->jlist_head);
> +		if (rc < 0)
> +			return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +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..98cfbb4602e1 100644
> --- a/cxl/event_trace.h
> +++ b/cxl/event_trace.h
> @@ -11,4 +11,11 @@ struct jlist_node {
>  	struct list_node list;
>  };
>  
> +struct event_ctx {
> +	const char **systems;
> +	struct list_head jlist_head;
> +};
> +
> +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)
> 
> 

[PATCH 3/7] ndctl: cxl: add common function to enable event trace

[Dave Jiang]

Add a common function for cxl command to enable event tracing for the
instance created. The interested "systems" will be enabled for tracing
as well.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/event_trace.c |   27 +++++++++++++++++++++++++++
 cxl/event_trace.h |    1 +
 2 files changed, 28 insertions(+)

diff --git a/cxl/event_trace.c b/cxl/event_trace.c
index d5e67b55a4d8..39ca5a9c49be 100644
--- a/cxl/event_trace.c
+++ b/cxl/event_trace.c
@@ -199,3 +199,30 @@ int cxl_parse_events(struct tracefs_instance *inst, struct event_ctx *ectx)
 	tep_free(tep);
 	return rc;
 }
+
+int cxl_event_tracing_enable(struct tracefs_instance *inst, const char **systems)
+{
+	int rc, i, en_count = 0;
+
+	for (i = 0; systems[i]; i++) {
+		char *path;
+
+		rc = asprintf(&path, "events/%s/enable", systems[i]);
+		if (rc == -1)
+			continue;
+		rc = tracefs_instance_file_write(inst, path, "1");
+		free(path);
+		if (rc == -1)
+			continue;
+		en_count++;
+	}
+
+	if (!en_count)
+		return -ENOENT;
+
+	if (tracefs_trace_is_on(inst))
+		return 0;
+
+	tracefs_trace_on(inst);
+	return 0;
+}
diff --git a/cxl/event_trace.h b/cxl/event_trace.h
index 98cfbb4602e1..b4453c393e7a 100644
--- a/cxl/event_trace.h
+++ b/cxl/event_trace.h
@@ -17,5 +17,6 @@ struct event_ctx {
 };
 
 int cxl_parse_events(struct tracefs_instance *inst, struct event_ctx *ectx);
+int cxl_event_tracing_enable(struct tracefs_instance *inst, const char **systems);
 
 #endif

[PATCH 4/7] ndctl: cxl: add common function to disable event trace

[Dave Jiang]

Add a common function for cxl command that disables the event trace for the
instance created.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/event_trace.c |    8 ++++++++
 cxl/event_trace.h |    1 +
 2 files changed, 9 insertions(+)

diff --git a/cxl/event_trace.c b/cxl/event_trace.c
index 39ca5a9c49be..50f9cc428aa7 100644
--- a/cxl/event_trace.c
+++ b/cxl/event_trace.c
@@ -226,3 +226,11 @@ int cxl_event_tracing_enable(struct tracefs_instance *inst, const char **systems
 	tracefs_trace_on(inst);
 	return 0;
 }
+
+int cxl_event_tracing_disable(struct tracefs_instance *inst)
+{
+	if (!tracefs_trace_is_on(inst))
+		return 0;
+
+	return tracefs_trace_off(inst);
+}
diff --git a/cxl/event_trace.h b/cxl/event_trace.h
index b4453c393e7a..9931fb32b245 100644
--- a/cxl/event_trace.h
+++ b/cxl/event_trace.h
@@ -18,5 +18,6 @@ struct event_ctx {
 
 int cxl_parse_events(struct tracefs_instance *inst, struct event_ctx *ectx);
 int cxl_event_tracing_enable(struct tracefs_instance *inst, const char **systems);
+int cxl_event_tracing_disable(struct tracefs_instance *inst);
 
 #endif

[PATCH 5/7] ndctl: cxl: add monitor function for event trace events

[Dave Jiang]

Add function that creates an event trace instance and utilize the cxl event
trace common functions to extract interested events from the trace buffer.
The monitoring function will pend on an epoll fd and wait for new events
to appear in the trace buffer.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/meson.build |    1 
 cxl/monitor.c   |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 140 insertions(+)
 create mode 100644 cxl/monitor.c

diff --git a/cxl/meson.build b/cxl/meson.build
index c59876262e76..eb8b2b1070ed 100644
--- a/cxl/meson.build
+++ b/cxl/meson.build
@@ -8,6 +8,7 @@ cxl_src = [
   'json.c',
   'filter.c',
   'event_trace.c',
+  'monitor.c',
 ]
 
 cxl_tool = executable('cxl',
diff --git a/cxl/monitor.c b/cxl/monitor.c
new file mode 100644
index 000000000000..2ab2e249f575
--- /dev/null
+++ b/cxl/monitor.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2022, Intel Corp. All rights reserved.
+/* Some bits copied from ndctl monitor code */
+#include <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+#include <json-c/json.h>
+#include <libgen.h>
+#include <time.h>
+#include <dirent.h>
+#include <ccan/list/list.h>
+#include <util/json.h>
+#include <util/util.h>
+#include <util/parse-options.h>
+#include <util/parse-configs.h>
+#include <util/strbuf.h>
+#include <sys/epoll.h>
+#include <sys/stat.h>
+#include <traceevent/event-parse.h>
+#include <tracefs/tracefs.h>
+#include <cxl/libcxl.h>
+
+/* reuse the core log helpers for the monitor logger */
+#ifndef ENABLE_LOGGING
+#define ENABLE_LOGGING
+#endif
+#ifndef ENABLE_DEBUG
+#define ENABLE_DEBUG
+#endif
+#include <util/log.h>
+
+#include "event_trace.h"
+
+const char *systems[] = { "cxl_events", "cxl_aer", NULL };
+
+static struct monitor {
+	struct log_ctx ctx;
+	FILE *log_file;
+	bool human;
+} monitor;
+
+static int monitor_event(struct cxl_ctx *ctx)
+{
+	int fd, epollfd, rc = 0, timeout = -1;
+	struct epoll_event ev, *events;
+	struct tracefs_instance *inst;
+	struct event_ctx ectx;
+	int jflag;
+
+	events = calloc(1, sizeof(struct epoll_event));
+	if (!events) {
+		err(&monitor, "alloc for events error\n");
+		return -ENOMEM;
+	}
+
+	epollfd = epoll_create1(0);
+	if (epollfd == -1) {
+		rc = -errno;
+		err(&monitor, "epoll_create1() error: %d\n", rc);
+		goto epoll_err;
+	}
+
+	inst = tracefs_instance_create("cxl_monitor");
+	if (!inst) {
+		rc = -errno;
+		err(&monitor, "tracefs_instance_crate( failed: %d\n", rc);
+		goto inst_err;
+	}
+
+	fd = tracefs_instance_file_open(inst, "trace_pipe", -1);
+	if (fd < 0) {
+		rc = fd;
+		err(&monitor, "tracefs_instance_file_open() err: %d\n", rc);
+		goto inst_file_err;
+	}
+
+	memset(&ev, 0, sizeof(ev));
+	ev.events = EPOLLIN;
+	ev.data.fd = fd;
+
+	if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev) != 0) {
+		rc = -errno;
+		err(&monitor, "epoll_ctl() error: %d\n", rc);
+		goto epoll_ctl_err;
+	}
+
+	rc = cxl_event_tracing_enable(inst, systems);
+	if (rc < 0) {
+		err(&monitor, "cxl_trace_event_enable() failed: %d\n", rc);
+		goto event_en_err;
+	}
+
+	ectx.systems = systems;
+	if (monitor.human)
+		jflag = JSON_C_TO_STRING_PRETTY;
+	else
+		jflag = JSON_C_TO_STRING_PLAIN;
+
+	while (1) {
+		struct jlist_node *jnode, *next;
+
+		rc = epoll_wait(epollfd, events, 1, timeout);
+		if (rc < 0) {
+			rc = -errno;
+			if (errno != EINTR)
+				err(&monitor, "epoll_wait error: %d\n", -errno);
+			break;
+		}
+
+		list_head_init(&ectx.jlist_head);
+		rc = cxl_parse_events(inst, &ectx);
+		if (rc < 0)
+			goto parse_err;
+
+		if (list_empty(&ectx.jlist_head))
+			continue;
+
+		list_for_each_safe(&ectx.jlist_head, jnode, next, list) {
+			notice(&monitor, "%s\n",
+				json_object_to_json_string_ext(jnode->jobj, jflag));
+			list_del(&jnode->list);
+			json_object_put(jnode->jobj);
+			free(jnode);
+		}
+	}
+
+parse_err:
+	rc = cxl_event_tracing_disable(inst);
+event_en_err:
+epoll_ctl_err:
+	close(fd);
+inst_file_err:
+	tracefs_instance_free(inst);
+inst_err:
+	close(epollfd);
+epoll_err:
+	free(events);
+	return rc;
+}

Re: [PATCH 5/7] ndctl: cxl: add monitor function for event trace events

[Alison Schofield]

On Wed, Sep 14, 2022 at 01:48:18PM -0700, Dave Jiang wrote:
> Add function that creates an event trace instance and utilize the cxl event
> trace common functions to extract interested events from the trace buffer.
> The monitoring function will pend on an epoll fd and wait for new events
> to appear in the trace buffer.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  cxl/meson.build |    1 
>  cxl/monitor.c   |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 140 insertions(+)
>  create mode 100644 cxl/monitor.c
> 
> diff --git a/cxl/meson.build b/cxl/meson.build
> index c59876262e76..eb8b2b1070ed 100644
> --- a/cxl/meson.build
> +++ b/cxl/meson.build
> @@ -8,6 +8,7 @@ cxl_src = [
>    'json.c',
>    'filter.c',
>    'event_trace.c',
> +  'monitor.c',
>  ]
>  
>  cxl_tool = executable('cxl',
> diff --git a/cxl/monitor.c b/cxl/monitor.c
> new file mode 100644
> index 000000000000..2ab2e249f575
> --- /dev/null
> +++ b/cxl/monitor.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2022, Intel Corp. All rights reserved.
> +/* Some bits copied from ndctl monitor code */
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <json-c/json.h>
> +#include <libgen.h>
> +#include <time.h>
> +#include <dirent.h>
> +#include <ccan/list/list.h>
> +#include <util/json.h>
> +#include <util/util.h>
> +#include <util/parse-options.h>
> +#include <util/parse-configs.h>
> +#include <util/strbuf.h>
> +#include <sys/epoll.h>
> +#include <sys/stat.h>
> +#include <traceevent/event-parse.h>
> +#include <tracefs/tracefs.h>
> +#include <cxl/libcxl.h>
> +
> +/* reuse the core log helpers for the monitor logger */
> +#ifndef ENABLE_LOGGING
> +#define ENABLE_LOGGING
> +#endif
> +#ifndef ENABLE_DEBUG
> +#define ENABLE_DEBUG
> +#endif
> +#include <util/log.h>
> +
> +#include "event_trace.h"
> +
> +const char *systems[] = { "cxl_events", "cxl_aer", NULL };
> +

Hi Dave,

So - I'm co-mingling feedback to this patch, with Ira's events patch,
and the poison patches I have in progress. We're all in flight, and
this seems like a good sync point.

I expected all CXL trace events will be defined in one header file:
include/trace/event/cxl.h

The enabling would spring, per event type, from there - so, to
enable 'cxl_aer' 

echo 1 > /sys/kernel/debug/tracing/events/cxl/cxl_aer/enable

I think your definition of *systems[] probably stands, but needs to
be prefixed with 'cxl/' and then filtered same.

Alison

> +static struct monitor {
> +	struct log_ctx ctx;
> +	FILE *log_file;
> +	bool human;
> +} monitor;
> +
> +static int monitor_event(struct cxl_ctx *ctx)
> +{
> +	int fd, epollfd, rc = 0, timeout = -1;
> +	struct epoll_event ev, *events;
> +	struct tracefs_instance *inst;
> +	struct event_ctx ectx;
> +	int jflag;
> +
> +	events = calloc(1, sizeof(struct epoll_event));
> +	if (!events) {
> +		err(&monitor, "alloc for events error\n");
> +		return -ENOMEM;
> +	}
> +
> +	epollfd = epoll_create1(0);
> +	if (epollfd == -1) {
> +		rc = -errno;
> +		err(&monitor, "epoll_create1() error: %d\n", rc);
> +		goto epoll_err;
> +	}
> +
> +	inst = tracefs_instance_create("cxl_monitor");
> +	if (!inst) {
> +		rc = -errno;
> +		err(&monitor, "tracefs_instance_crate( failed: %d\n", rc);
> +		goto inst_err;
> +	}
> +
> +	fd = tracefs_instance_file_open(inst, "trace_pipe", -1);
> +	if (fd < 0) {
> +		rc = fd;
> +		err(&monitor, "tracefs_instance_file_open() err: %d\n", rc);
> +		goto inst_file_err;
> +	}
> +
> +	memset(&ev, 0, sizeof(ev));
> +	ev.events = EPOLLIN;
> +	ev.data.fd = fd;
> +
> +	if (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev) != 0) {
> +		rc = -errno;
> +		err(&monitor, "epoll_ctl() error: %d\n", rc);
> +		goto epoll_ctl_err;
> +	}
> +
> +	rc = cxl_event_tracing_enable(inst, systems);
> +	if (rc < 0) {
> +		err(&monitor, "cxl_trace_event_enable() failed: %d\n", rc);
> +		goto event_en_err;
> +	}
> +
> +	ectx.systems = systems;
> +	if (monitor.human)
> +		jflag = JSON_C_TO_STRING_PRETTY;
> +	else
> +		jflag = JSON_C_TO_STRING_PLAIN;
> +
> +	while (1) {
> +		struct jlist_node *jnode, *next;
> +
> +		rc = epoll_wait(epollfd, events, 1, timeout);
> +		if (rc < 0) {
> +			rc = -errno;
> +			if (errno != EINTR)
> +				err(&monitor, "epoll_wait error: %d\n", -errno);
> +			break;
> +		}
> +
> +		list_head_init(&ectx.jlist_head);
> +		rc = cxl_parse_events(inst, &ectx);
> +		if (rc < 0)
> +			goto parse_err;
> +
> +		if (list_empty(&ectx.jlist_head))
> +			continue;
> +
> +		list_for_each_safe(&ectx.jlist_head, jnode, next, list) {
> +			notice(&monitor, "%s\n",
> +				json_object_to_json_string_ext(jnode->jobj, jflag));
> +			list_del(&jnode->list);
> +			json_object_put(jnode->jobj);
> +			free(jnode);
> +		}
> +	}
> +
> +parse_err:
> +	rc = cxl_event_tracing_disable(inst);
> +event_en_err:
> +epoll_ctl_err:
> +	close(fd);
> +inst_file_err:
> +	tracefs_instance_free(inst);
> +inst_err:
> +	close(epollfd);
> +epoll_err:
> +	free(events);
> +	return rc;
> +}
> 
> 

[PATCH 6/7] ndctl: cxl: add logging functions for monitor

[Dave Jiang]

Duplicate log functions from ndctl/monitor to use for stdout and file
logging.

Signed-off-by: Dave Jiang <dave.jiang@gmail.com>
---
 cxl/monitor.c |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/cxl/monitor.c b/cxl/monitor.c
index 2ab2e249f575..e030542a3fe8 100644
--- a/cxl/monitor.c
+++ b/cxl/monitor.c
@@ -39,6 +39,32 @@ static struct monitor {
 	bool human;
 } monitor;
 
+static void log_standard(struct log_ctx *ctx, int priority, const char *file,
+		int line, const char *fn, const char *format, va_list args)
+{
+	if (priority == 6)
+		vfprintf(stdout, format, args);
+	else
+		vfprintf(stderr, format, args);
+}
+
+static void log_file(struct log_ctx *ctx, int priority, const char *file,
+		int line, const char *fn, const char *format, va_list args)
+{
+	FILE *f = monitor.log_file;
+
+	if (priority != LOG_NOTICE) {
+		struct timespec ts;
+
+		clock_gettime(CLOCK_REALTIME, &ts);
+		fprintf(f, "[%10ld.%09ld] [%d] ", ts.tv_sec, ts.tv_nsec, getpid());
+		vfprintf(f, format, args);
+	} else
+		vfprintf(f, format, args);
+
+	fflush(f);
+}
+
 static int monitor_event(struct cxl_ctx *ctx)
 {
 	int fd, epollfd, rc = 0, timeout = -1;

Re: [PATCH 6/7] ndctl: cxl: add logging functions for monitor

[Nathan Fontenot]

On 9/14/22 15:48, Dave Jiang wrote:
> Duplicate log functions from ndctl/monitor to use for stdout and file
> logging.
> 
> Signed-off-by: Dave Jiang <dave.jiang@gmail.com>
> ---
>  cxl/monitor.c |   26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/cxl/monitor.c b/cxl/monitor.c
> index 2ab2e249f575..e030542a3fe8 100644
> --- a/cxl/monitor.c
> +++ b/cxl/monitor.c
> @@ -39,6 +39,32 @@ static struct monitor {
>  	bool human;
>  } monitor;
>  
> +static void log_standard(struct log_ctx *ctx, int priority, const char *file,
> +		int line, const char *fn, const char *format, va_list args)
> +{
> +	if (priority == 6)
> +		vfprintf(stdout, format, args);
> +	else
> +		vfprintf(stderr, format, args);
> +}
> +
> +static void log_file(struct log_ctx *ctx, int priority, const char *file,
> +		int line, const char *fn, const char *format, va_list args)
> +{
> +	FILE *f = monitor.log_file;
> +
> +	if (priority != LOG_NOTICE) {
> +		struct timespec ts;
> +
> +		clock_gettime(CLOCK_REALTIME, &ts);
> +		fprintf(f, "[%10ld.%09ld] [%d] ", ts.tv_sec, ts.tv_nsec, getpid());
> +		vfprintf(f, format, args);
> +	} else
> +		vfprintf(f, format, args);

The vfprintf() is always done here, you could just move it outside the if/else block.

-Nathan

> +
> +	fflush(f);
> +}
> +
>  static int monitor_event(struct cxl_ctx *ctx)
>  {
>  	int fd, epollfd, rc = 0, timeout = -1;
> 
> 

Re: [PATCH 6/7] ndctl: cxl: add logging functions for monitor

[Dave Jiang]

On 9/15/2022 8:55 AM, Nathan Fontenot wrote:
> On 9/14/22 15:48, Dave Jiang wrote:
>> Duplicate log functions from ndctl/monitor to use for stdout and file
>> logging.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@gmail.com>
>> ---
>>   cxl/monitor.c |   26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/cxl/monitor.c b/cxl/monitor.c
>> index 2ab2e249f575..e030542a3fe8 100644
>> --- a/cxl/monitor.c
>> +++ b/cxl/monitor.c
>> @@ -39,6 +39,32 @@ static struct monitor {
>>   	bool human;
>>   } monitor;
>>   
>> +static void log_standard(struct log_ctx *ctx, int priority, const char *file,
>> +		int line, const char *fn, const char *format, va_list args)
>> +{
>> +	if (priority == 6)
>> +		vfprintf(stdout, format, args);
>> +	else
>> +		vfprintf(stderr, format, args);
>> +}
>> +
>> +static void log_file(struct log_ctx *ctx, int priority, const char *file,
>> +		int line, const char *fn, const char *format, va_list args)
>> +{
>> +	FILE *f = monitor.log_file;
>> +
>> +	if (priority != LOG_NOTICE) {
>> +		struct timespec ts;
>> +
>> +		clock_gettime(CLOCK_REALTIME, &ts);
>> +		fprintf(f, "[%10ld.%09ld] [%d] ", ts.tv_sec, ts.tv_nsec, getpid());
>> +		vfprintf(f, format, args);
>> +	} else
>> +		vfprintf(f, format, args);
> The vfprintf() is always done here, you could just move it outside the if/else block.

Thanks! Will move. I just blindly copied the code from ndctl/monitor.c.


>
> -Nathan
>
>> +
>> +	fflush(f);
>> +}
>> +
>>   static int monitor_event(struct cxl_ctx *ctx)
>>   {
>>   	int fd, epollfd, rc = 0, timeout = -1;
>>
>>

[PATCH 7/7] ndctl: cxl: add monitor command to cxl

[Dave Jiang]

Connect the monitoring functionality to the cxl monitor command. Add basic
functionality to the cxl monitor command where it can be launched as a daemon
and logging can be designated to stdout or a file.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/builtin.h |    1 +
 cxl/cxl.c     |    1 +
 cxl/monitor.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+)

diff --git a/cxl/builtin.h b/cxl/builtin.h
index b28c2213993b..34c5cfb49051 100644
--- a/cxl/builtin.h
+++ b/cxl/builtin.h
@@ -22,4 +22,5 @@ int cmd_create_region(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_enable_region(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_disable_region(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_destroy_region(int argc, const char **argv, struct cxl_ctx *ctx);
+int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx);
 #endif /* _CXL_BUILTIN_H_ */
diff --git a/cxl/cxl.c b/cxl/cxl.c
index dd1be7a054a1..3be7026f43d3 100644
--- a/cxl/cxl.c
+++ b/cxl/cxl.c
@@ -76,6 +76,7 @@ static struct cmd_struct commands[] = {
 	{ "enable-region", .c_fn = cmd_enable_region },
 	{ "disable-region", .c_fn = cmd_disable_region },
 	{ "destroy-region", .c_fn = cmd_destroy_region },
+	{ "monitor", .c_fn = cmd_monitor },
 };
 
 int main(int argc, const char **argv)
diff --git a/cxl/monitor.c b/cxl/monitor.c
index e030542a3fe8..e185d15254ec 100644
--- a/cxl/monitor.c
+++ b/cxl/monitor.c
@@ -34,9 +34,12 @@
 const char *systems[] = { "cxl_events", "cxl_aer", NULL };
 
 static struct monitor {
+	const char *log;
 	struct log_ctx ctx;
 	FILE *log_file;
 	bool human;
+	bool verbose;
+	bool daemon;
 } monitor;
 
 static void log_standard(struct log_ctx *ctx, int priority, const char *file,
@@ -163,3 +166,70 @@ epoll_err:
 	free(events);
 	return rc;
 }
+
+int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx)
+{
+	const struct option options[] = {
+		OPT_FILENAME('l', "log", &monitor.log,
+				"<file> | standard",
+				"where to output the monitor's notification"),
+		OPT_BOOLEAN('\0', "daemon", &monitor.daemon,
+				"run cxl monitor as a daemon"),
+		OPT_BOOLEAN('u', "human", &monitor.human,
+				"use human friendly output formats"),
+		OPT_BOOLEAN('v', "verbose", &monitor.verbose,
+				"emit extra debug messages to log"),
+		OPT_END(),
+	};
+	const char * const u[] = {
+		"cxl monitor [<options>]",
+		NULL
+	};
+	const char *prefix ="./";
+	int rc = 0, i;
+
+	argc = parse_options_prefix(argc, argv, prefix, options, u, 0);
+	for (i = 0; i < argc; i++)
+		error("unknown parameter \"%s\"\n", argv[i]);
+	if (argc)
+		usage_with_options(u, options);
+
+	log_init(&monitor.ctx, "cxl/monitor", "CXL_MONITOR_LOG");
+	monitor.ctx.log_fn = log_standard;
+
+	if (monitor.verbose)
+		monitor.ctx.log_priority = LOG_DEBUG;
+	else
+		monitor.ctx.log_priority = LOG_INFO;
+
+	if (monitor.log) {
+		if (strncmp(monitor.log, "./", 2) != 0)
+			fix_filename(prefix, (const char **)&monitor.log);
+		if (strncmp(monitor.log, "./standard", 10) == 0) {
+			monitor.ctx.log_fn = log_standard;
+		} else {
+			monitor.log_file = fopen(monitor.log, "a+");
+			if (!monitor.log_file) {
+				rc = -errno;
+				error("open %s failed: %d\n", monitor.log, rc);
+				goto out;
+			}
+			monitor.ctx.log_fn = log_file;
+		}
+	}
+
+	if (monitor.daemon) {
+		if (daemon(0, 0) != 0) {
+			err(&monitor, "daemon start failed\n");
+			goto out;
+		}
+		info(&monitor, "cxl monitor daemon started.\n");
+	}
+
+	rc = monitor_event(ctx);
+
+out:
+	if (monitor.log_file)
+		fclose(monitor.log_file);
+	return rc;
+}

Re: [PATCH 0/7] ndctl: cxl: add monitor support for trace events

[Dave Jiang]

On 9/14/2022 1:47 PM, Dave Jiang wrote:
> This patch series for ndctl implements the monitor command for the cxl tool.
> The initial implementation will collect CXL trace events emitted by the
> kernel. libtraceevent and libtracefs will be used to parse the trace
> event buffer. The monitor will pend on an epoll fd and wait for new event
> entries to be posted. The output will be in json format. By default the events
> are emitted to stdio, but can also be logged to a file. Each event is converted
> to a JSON object and logged as such. All the fields exported are read by the
> monitor code and added to the JSON object.
>
> ---
>
> Dave Jiang (7):
>        ndctl: cxl: add helper function to parse trace event to json object
>        ndctl: cxl: add helper to parse through all current events
>        ndctl: cxl: add common function to enable event trace
>        ndctl: cxl: add common function to disable event trace
>        ndctl: cxl: add monitor function for event trace events
>        ndctl: cxl: add logging functions for monitor
>        ndctl: cxl: add monitor command to cxl

Missing man page and systemd service. Will add in v2. Also will add 
default log file path when running in daemon mode.


>
>   cxl/builtin.h     |   1 +
>   cxl/cxl.c         |   1 +
>   cxl/event_trace.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++
>   cxl/event_trace.h |  23 +++++
>   cxl/meson.build   |   4 +
>   cxl/monitor.c     | 235 +++++++++++++++++++++++++++++++++++++++++++++
>   meson.build       |   3 +
>   7 files changed, 503 insertions(+)
>   create mode 100644 cxl/event_trace.c
>   create mode 100644 cxl/event_trace.h
>   create mode 100644 cxl/monitor.c
>
> --
>

Re: [PATCH 0/7] ndctl: cxl: add monitor support for trace events

[Verma, Vishal L]

On Wed, 2022-09-14 at 15:19 -0700, Dave Jiang wrote:
> 
> On 9/14/2022 1:47 PM, Dave Jiang wrote:
> > This patch series for ndctl implements the monitor command for the cxl tool.
> > The initial implementation will collect CXL trace events emitted by the
> > kernel. libtraceevent and libtracefs will be used to parse the trace
> > event buffer. The monitor will pend on an epoll fd and wait for new event
> > entries to be posted. The output will be in json format. By default the events
> > are emitted to stdio, but can also be logged to a file. Each event is converted
> > to a JSON object and logged as such. All the fields exported are read by the
> > monitor code and added to the JSON object.
> > 
> > ---
> > 
> > Dave Jiang (7):
> >        ndctl: cxl: add helper function to parse trace event to json object
> >        ndctl: cxl: add helper to parse through all current events
> >        ndctl: cxl: add common function to enable event trace
> >        ndctl: cxl: add common function to disable event trace
> >        ndctl: cxl: add monitor function for event trace events
> >        ndctl: cxl: add logging functions for monitor
> >        ndctl: cxl: add monitor command to cxl
> 
> Missing man page and systemd service. Will add in v2. Also will add 
> default log file path when running in daemon mode.

Haven't started looking at the rest, but I'll quickly add that no need
to include the 'ndctl:' prefix for these. 'cxl:' (or 'libcxl:' for
library patches) should be enough.

> > 
> >   cxl/builtin.h     |   1 +
> >   cxl/cxl.c         |   1 +
> >   cxl/event_trace.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++
> >   cxl/event_trace.h |  23 +++++
> >   cxl/meson.build   |   4 +
> >   cxl/monitor.c     | 235 +++++++++++++++++++++++++++++++++++++++++++++
> >   meson.build       |   3 +
> >   7 files changed, 503 insertions(+)
> >   create mode 100644 cxl/event_trace.c
> >   create mode 100644 cxl/event_trace.h
> >   create mode 100644 cxl/monitor.c
> > 
> > --
> > 

Re: [PATCH 0/7] ndctl: cxl: add monitor support for trace events

[Jonathan Cameron]

On Wed, 14 Sep 2022 13:47:49 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> This patch series for ndctl implements the monitor command for the cxl tool.
> The initial implementation will collect CXL trace events emitted by the
> kernel. libtraceevent and libtracefs will be used to parse the trace
> event buffer. The monitor will pend on an epoll fd and wait for new event
> entries to be posted. The output will be in json format. By default the events
> are emitted to stdio, but can also be logged to a file. Each event is converted
> to a JSON object and logged as such. All the fields exported are read by the
> monitor code and added to the JSON object.

Hi Dave,

FYI, I just wanted to mention that we will be aiming to duplicate a bunch
of this functionality in rasdaemon.

As that is handling the equivalent of many of these events on our systems
from other components we don't want to grab the data via alternative tooling
and then have to merge it later.

Absolutely not a problem to have it in this tool as well. I mostly wanted
to raise it because longer term we'll need to be careful to test with both
solutions if there are any changes to what they are being sent.

Thanks,

Jonathan

> 
> ---
> 
> Dave Jiang (7):
>       ndctl: cxl: add helper function to parse trace event to json object
>       ndctl: cxl: add helper to parse through all current events
>       ndctl: cxl: add common function to enable event trace
>       ndctl: cxl: add common function to disable event trace
>       ndctl: cxl: add monitor function for event trace events
>       ndctl: cxl: add logging functions for monitor
>       ndctl: cxl: add monitor command to cxl
> 
> 
>  cxl/builtin.h     |   1 +
>  cxl/cxl.c         |   1 +
>  cxl/event_trace.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++
>  cxl/event_trace.h |  23 +++++
>  cxl/meson.build   |   4 +
>  cxl/monitor.c     | 235 +++++++++++++++++++++++++++++++++++++++++++++
>  meson.build       |   3 +
>  7 files changed, 503 insertions(+)
>  create mode 100644 cxl/event_trace.c
>  create mode 100644 cxl/event_trace.h
>  create mode 100644 cxl/monitor.c
> 
> --
> 

Re: [PATCH 0/7] ndctl: cxl: add monitor support for trace events

[Dave Jiang]

On 9/22/2022 7:21 AM, Jonathan Cameron wrote:
> On Wed, 14 Sep 2022 13:47:49 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
>
>> This patch series for ndctl implements the monitor command for the cxl tool.
>> The initial implementation will collect CXL trace events emitted by the
>> kernel. libtraceevent and libtracefs will be used to parse the trace
>> event buffer. The monitor will pend on an epoll fd and wait for new event
>> entries to be posted. The output will be in json format. By default the events
>> are emitted to stdio, but can also be logged to a file. Each event is converted
>> to a JSON object and logged as such. All the fields exported are read by the
>> monitor code and added to the JSON object.
> Hi Dave,
>
> FYI, I just wanted to mention that we will be aiming to duplicate a bunch
> of this functionality in rasdaemon.
>
> As that is handling the equivalent of many of these events on our systems
> from other components we don't want to grab the data via alternative tooling
> and then have to merge it later.
>
> Absolutely not a problem to have it in this tool as well. I mostly wanted
> to raise it because longer term we'll need to be careful to test with both
> solutions if there are any changes to what they are being sent.

Thanks for the heads up Jonathan!


>
> Thanks,
>
> Jonathan
>
>> ---
>>
>> Dave Jiang (7):
>>        ndctl: cxl: add helper function to parse trace event to json object
>>        ndctl: cxl: add helper to parse through all current events
>>        ndctl: cxl: add common function to enable event trace
>>        ndctl: cxl: add common function to disable event trace
>>        ndctl: cxl: add monitor function for event trace events
>>        ndctl: cxl: add logging functions for monitor
>>        ndctl: cxl: add monitor command to cxl
>>
>>
>>   cxl/builtin.h     |   1 +
>>   cxl/cxl.c         |   1 +
>>   cxl/event_trace.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++
>>   cxl/event_trace.h |  23 +++++
>>   cxl/meson.build   |   4 +
>>   cxl/monitor.c     | 235 +++++++++++++++++++++++++++++++++++++++++++++
>>   meson.build       |   3 +
>>   7 files changed, 503 insertions(+)
>>   create mode 100644 cxl/event_trace.c
>>   create mode 100644 cxl/event_trace.h
>>   create mode 100644 cxl/monitor.c
>>
>> --
>>