Re: [PATCH v3 05/10] cxl: add monitor function for event trace events

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

On 11/2/2022 11:06 PM, Steven Rostedt wrote:
> On Wed, 02 Nov 2022 14:20:32 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
>> ---
>>   cxl/meson.build |    1
>>   cxl/monitor.c   |  140 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 141 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..85559d9a4b94
>> --- /dev/null
>> +++ b/cxl/monitor.c
>> @@ -0,0 +1,140 @@
>> +// 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"
>> +
>> +static const char *cxl_system = "cxl";
>> +
>> +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);
> 
>   "crate"? Been coding a bit too much Rust lately?

Ooops. Will fix.

> 
>> +		goto inst_err;
>> +	}
>> +
>> +	fd = tracefs_instance_file_open(inst, "trace_pipe", -1);
> 
> I'm curious to why you are opening trace_pipe?
> 
>> +	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;
> 
> Is it a way to know if there's something to read?

Yes. Since trace_pipe() is the read once data stream, seems like the 
right place to put the epoll on. Is that the wrong way to do this?

> 
> -- Steve
> 
>> +
>> +	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, cxl_system);
>> +	if (rc < 0) {
>> +		err(&monitor, "cxl_trace_event_enable() failed: %d\n", rc);
>> +		goto event_en_err;
>> +	}
>> +
>> +	memset(&ectx, 0, sizeof(ectx));
>> +	ectx.system = cxl_system;
>> +	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;
>> +}
>>
>