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

[Alison Schofield <alison.schofield@intel.com>]

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;
> +}
> 
>