Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
* [ltt-dev] [UST PATCH 1/3] Add trace_event structs
@ 2010-09-08 17:52 Nils Carlson
  2010-09-08 17:52 ` [ltt-dev] [UST PATCH 2/3] Add trace_event library initialisation Nils Carlson
  2010-09-08 17:52 ` [ltt-dev] [UST PATCH 3/3] Add functions and command line for listing trace_events Nils Carlson
  0 siblings, 2 replies; 4+ messages in thread
From: Nils Carlson @ 2010-09-08 17:52 UTC (permalink / raw)


Add trace_event structs, registration functions and trace_event
iterator and locking functions.

Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
---
 include/ust/tracepoint.h |   57 +++++++++++++++++
 include/ust/ust_trace.h  |   16 +++++
 libust/Makefile.am       |    1 +
 libust/trace_event.c     |  157 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 231 insertions(+), 0 deletions(-)
 create mode 100644 libust/trace_event.c

diff --git a/include/ust/tracepoint.h b/include/ust/tracepoint.h
index 9c7c091..1c4a384 100644
--- a/include/ust/tracepoint.h
+++ b/include/ust/tracepoint.h
@@ -337,6 +337,63 @@ extern int tracepoint_unregister_lib(struct tracepoint *tracepoints_start);
  * TRACE_EVENT_FN to perform any (un)registration work.
  */
 
+struct trace_event {
+	const char *name;
+	int (*regfunc)(void *data);
+	int (*unregfunc)(void *data);
+} __attribute__((aligned(32)));
+
+struct trace_event_lib {
+	struct trace_event *trace_events_start;
+	int trace_events_count;
+	struct list_head list;
+};
+
+struct trace_event_iter {
+	struct trace_event_lib *lib;
+	struct trace_event *trace_event;
+};
+
+extern void lock_trace_events(void);
+extern void unlock_trace_events(void);
+
+extern void trace_event_iter_start(struct trace_event_iter *iter);
+extern void trace_event_iter_next(struct trace_event_iter *iter);
+extern void trace_event_iter_reset(struct trace_event_iter *iter);
+
+extern int trace_event_get_iter_range(struct trace_event **trace_event,
+				      struct trace_event *begin,
+				      struct trace_event *end);
+
+extern void trace_event_update_process(void);
+extern int is_trace_event_enabled(const char *channel, const char *name);
+
+extern int trace_event_register_lib(struct trace_event *start_trace_events,
+				    int trace_event_count);
+
+extern int trace_event_unregister_lib(struct trace_event *start_trace_events);
+
+#define TRACE_EVENT_LIB							\
+	extern struct trace_event __start___trace_events[]		\
+	__attribute__((weak, visibility("hidden")));			\
+	extern struct trace_event __stop___trace_events[]		\
+	__attribute__((weak, visibility("hidden")));			\
+	static void __attribute__((constructor))			\
+	__trace_events__init(void)					\
+	{								\
+		long trace_event_count =((long)__stop___trace_events-	\
+					 (long)__start___trace_events)	\
+			/sizeof(struct trace_event);			\
+		trace_event_register_lib(__start___trace_events,	\
+					 trace_event_count);		\
+	}								\
+									\
+	static void __attribute__((destructor))				\
+	__trace_event__destroy(void)					\
+	{								\
+		trace_event_unregister_lib(__start___trace_events);	\
+	}
+
 #define DECLARE_TRACE_EVENT_CLASS(name, proto, args, tstruct, assign, print)
 #define DEFINE_TRACE_EVENT(template, name, proto, args)		\
 	DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
diff --git a/include/ust/ust_trace.h b/include/ust/ust_trace.h
index e1b0257..01c241d 100644
--- a/include/ust/ust_trace.h
+++ b/include/ust/ust_trace.h
@@ -23,6 +23,8 @@
  * to a printf
  */
 
+#include <stdio.h>
+
 /*
  * Stage 1. Create a struct and a printf calling function
  * that is connected to the tracepoint at load time.
@@ -66,6 +68,20 @@
 									\
 		printf(print);						\
 	}								\
+	static inline int register_event_##name(void *data)		\
+	{								\
+		return register_trace_##name(trace_printf_##name, data); \
+	}								\
+	static inline int unregister_event_##name(void *data)		\
+	{								\
+		return unregister_trace_##name(trace_printf_##name, data); \
+	}								\
+	struct trace_event __event_##name				\
+	__attribute__((section("__trace_events"), aligned(32)))	= {	\
+		__tpstrtab_##name,					\
+		register_event_##name,					\
+		unregister_event_##name					\
+	};								\
 	static void __attribute__((constructor)) init_##name()		\
 	{								\
 		void *dummy;						\
diff --git a/libust/Makefile.am b/libust/Makefile.am
index a6051a8..697b469 100644
--- a/libust/Makefile.am
+++ b/libust/Makefile.am
@@ -6,6 +6,7 @@ lib_LTLIBRARIES = libust.la
 libust_la_SOURCES = \
 	marker.c \
 	tracepoint.c \
+	trace_event.c \
 	channels.c \
 	channels.h \
 	marker-control.c \
diff --git a/libust/trace_event.c b/libust/trace_event.c
new file mode 100644
index 0000000..af1e3fb
--- /dev/null
+++ b/libust/trace_event.c
@@ -0,0 +1,157 @@
+/*
+ * Copyright (C) 2010 Nils Carlson
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
+ *
+ */
+
+#include <errno.h>
+#include <ust/tracepoint.h>
+#include <ust/core.h>
+#include <ust/kcompat/kcompat.h>
+#include "usterr.h"
+
+#define _LGPL_SOURCE
+#include <urcu-bp.h>
+
+/* libraries that contain trace_events (struct trace_event_lib) */
+static LIST_HEAD(libs);
+
+static DEFINE_MUTEX(trace_events_mutex);
+
+void lock_trace_events(void)
+{
+	pthread_mutex_lock(&trace_events_mutex);
+}
+
+void unlock_trace_events(void)
+{
+	pthread_mutex_unlock(&trace_events_mutex);
+}
+
+
+int lib_get_iter_trace_events(struct trace_event_iter *iter)
+{
+	struct trace_event_lib *iter_lib;
+	int found = 0;
+
+	list_for_each_entry(iter_lib, &libs, list) {
+		if (iter_lib < iter->lib)
+			continue;
+		else if (iter_lib > iter->lib)
+			iter->trace_event = NULL;
+		found = trace_event_get_iter_range(&iter->trace_event,
+			iter_lib->trace_events_start,
+			iter_lib->trace_events_start + iter_lib->trace_events_count);
+		if (found) {
+			iter->lib = iter_lib;
+			break;
+		}
+	}
+	return found;
+}
+
+/**
+ * trace_event_get_iter_range - Get a next trace_event iterator given a range.
+ * @trace_event: current trace_events (in), next trace_event (out)
+ * @begin: beginning of the range
+ * @end: end of the range
+ *
+ * Returns whether a next trace_event has been found (1) or not (0).
+ * Will return the first trace_event in the range if the input trace_event is NULL.
+ */
+int trace_event_get_iter_range(struct trace_event **trace_event, struct trace_event *begin,
+	struct trace_event *end)
+{
+	if (!*trace_event && begin != end) {
+		*trace_event = begin;
+		return 1;
+	}
+	if (*trace_event >= begin && *trace_event < end)
+		return 1;
+	return 0;
+}
+
+static void trace_event_get_iter(struct trace_event_iter *iter)
+{
+	int found = 0;
+
+	found = lib_get_iter_trace_events(iter);
+end:
+	if (!found)
+		trace_event_iter_reset(iter);
+}
+
+void trace_event_iter_start(struct trace_event_iter *iter)
+{
+	trace_event_get_iter(iter);
+}
+
+void trace_event_iter_next(struct trace_event_iter *iter)
+{
+	iter->trace_event++;
+	/*
+	 * iter->trace_event may be invalid because we blindly incremented it.
+	 * Make sure it is valid by marshalling on the trace_events, getting the
+	 * trace_events from following modules if necessary.
+	 */
+	trace_event_get_iter(iter);
+}
+
+void trace_event_iter_reset(struct trace_event_iter *iter)
+{
+	iter->lib = NULL;
+	iter->trace_event = NULL;
+}
+
+int trace_event_register_lib(struct trace_event *trace_events_start,
+			     int trace_events_count)
+{
+	struct trace_event_lib *pl;
+
+	pl = (struct trace_event_lib *) malloc(sizeof(struct trace_event_lib));
+
+	pl->trace_events_start = trace_events_start;
+	pl->trace_events_count = trace_events_count;
+
+	/* FIXME: maybe protect this with its own mutex? */
+	pthread_mutex_lock(&trace_events_mutex);
+	list_add(&pl->list, &libs);
+	pthread_mutex_unlock(&trace_events_mutex);
+
+	DBG("just registered a trace_events section from %p and having %d trace_events", trace_events_start, trace_events_count);
+
+	return 0;
+}
+
+int trace_event_unregister_lib(struct trace_event *trace_events_start)
+{
+	struct trace_event_lib *lib;
+
+	pthread_mutex_lock(&trace_events_mutex);
+
+	list_for_each_entry(lib, &libs, list) {
+		if(lib->trace_events_start == trace_events_start) {
+			struct trace_event_lib *lib2free = lib;
+			list_del(&lib->list);
+			free(lib2free);
+			break;
+		}
+	}
+
+	pthread_mutex_unlock(&trace_events_mutex);
+
+	return 0;
+}
-- 
1.7.1





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [ltt-dev] [UST PATCH 2/3] Add trace_event library initialisation
  2010-09-08 17:52 [ltt-dev] [UST PATCH 1/3] Add trace_event structs Nils Carlson
@ 2010-09-08 17:52 ` Nils Carlson
  2010-09-08 17:52 ` [ltt-dev] [UST PATCH 3/3] Add functions and command line for listing trace_events Nils Carlson
  1 sibling, 0 replies; 4+ messages in thread
From: Nils Carlson @ 2010-09-08 17:52 UTC (permalink / raw)


Add library initialisation with dummy trace_event in order
to create sections.

Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
---
 Makefile.am          |    4 ++--
 libust-initializer.c |    5 +++++
 libust-initializer.h |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 libust-initializer.h

diff --git a/Makefile.am b/Makefile.am
index c77ef8f..5132d59 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -7,7 +7,7 @@ ACLOCAL_AMFLAGS = -I config
 # the linker script.
 SUBDIRS = snprintf libustcomm libustcmd libust . tests libustinstr-malloc libustd ustd ustctl libustfork include doc
 
-EXTRA_DIST = libust.ldscript.in libust-initializer.c
+EXTRA_DIST = libust.ldscript.in libust-initializer.c libust-initializer.h
 dist_bin_SCRIPTS = usttrace
 
 ldscriptsdir = $(libdir)
@@ -21,4 +21,4 @@ libust.so: libust.ldscript.in
 # It is very important to compile the initializer with PIC otherwise we
 # may get obscure errors when linking to shared libraries.
 libust-initializer.o: libust-initializer.c
-	$(CC) $(CFLAGS) -fno-strict-aliasing -fPIC -c -I$(top_srcdir)/include -o $@ $<
+	$(CC) $(CFLAGS) -fno-strict-aliasing -fPIC -c -I$(top_srcdir)/include -I$(top_srcdir) -o $@ $<
diff --git a/libust-initializer.c b/libust-initializer.c
index 390218e..2ac5637 100644
--- a/libust-initializer.c
+++ b/libust-initializer.c
@@ -21,12 +21,17 @@
 DECLARE_TRACE(ust_dummytp, TP_PROTO(int anint), TP_ARGS(anint));
 DEFINE_TRACE(ust_dummytp);
 
+#define CREATE_TRACE_POINTS
+#include "libust-initializer.h"
+
 void dummy_libust_initializer_func(void)
 {
 	int i;
 	trace_mark(ust, dummymark, MARK_NOARGS);
 	trace_ust_dummytp(i);
+	trace_ust_dummy_event(i);
 }
 
 MARKER_LIB;
 TRACEPOINT_LIB;
+TRACE_EVENT_LIB;
diff --git a/libust-initializer.h b/libust-initializer.h
new file mode 100644
index 0000000..b5d88ac
--- /dev/null
+++ b/libust-initializer.h
@@ -0,0 +1,48 @@
+/* Copyright (C) 2010 Nils Carlson <nils.carlson at ericsson.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM ust_dummy
+
+#if !defined(_TRACE_EVENT_TEST_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_EVENT_TEST_H
+
+#include <ust/tracepoint.h>
+
+TRACE_EVENT(ust_dummy_event,
+
+	TP_PROTO(int dummy_int),
+
+	TP_ARGS(dummy_int),
+
+	TP_STRUCT__entry(
+		__field(	int,	dummy	)
+	),
+
+	TP_fast_assign(
+		__entry->dummy = dummy_int;
+	),
+
+	TP_printf("dummy=%d", __entry->dummy)
+);
+
+#endif /* _TRACE_EVENT_TEST_H */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_FILE libust-initializer
+#include <ust/define_trace.h>
-- 
1.7.1





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [ltt-dev] [UST PATCH 3/3] Add functions and command line for listing trace_events
  2010-09-08 17:52 [ltt-dev] [UST PATCH 1/3] Add trace_event structs Nils Carlson
  2010-09-08 17:52 ` [ltt-dev] [UST PATCH 2/3] Add trace_event library initialisation Nils Carlson
@ 2010-09-08 17:52 ` Nils Carlson
  2010-09-08 18:23   ` David Goulet
  1 sibling, 1 reply; 4+ messages in thread
From: Nils Carlson @ 2010-09-08 17:52 UTC (permalink / raw)



Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
---
 include/ust/ustcmd.h |    6 ++++
 libust/tracectl.c    |   34 ++++++++++++++++++++-
 libustcmd/ustcmd.c   |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++
 ustctl/ustctl.c      |   23 ++++++++++++++-
 4 files changed, 138 insertions(+), 3 deletions(-)

diff --git a/include/ust/ustcmd.h b/include/ust/ustcmd.h
index 60f5018..986ae61 100644
--- a/include/ust/ustcmd.h
+++ b/include/ust/ustcmd.h
@@ -43,6 +43,10 @@ struct marker_status {
 	char *fs; /* Format string (end of marker_status array if NULL) */
 };
 
+struct trace_event_status {
+	char *name;
+};
+
 extern pid_t *ustcmd_get_online_pids(void);
 extern int ustcmd_set_marker_state(const char *, int, pid_t);
 extern int ustcmd_set_subbuf_size(const char *, pid_t);
@@ -59,6 +63,8 @@ extern int ustcmd_free_cmsf(struct marker_status *);
 extern unsigned int ustcmd_count_nl(const char *);
 extern int ustcmd_send_cmd(const char *, pid_t, char **);
 extern int ustcmd_get_cmsf(struct marker_status **, pid_t);
+extern int ustcmd_free_tes(struct trace_event_status *);
+extern int ustcmd_get_tes(struct trace_event_status **, pid_t);
 extern int ustcmd_set_sock_path(const char *, pid_t);
 extern int ustcmd_get_sock_path(char **, pid_t);
 extern int ustcmd_force_switch(pid_t);
diff --git a/libust/tracectl.c b/libust/tracectl.c
index e64b26f..7dbc572 100644
--- a/libust/tracectl.c
+++ b/libust/tracectl.c
@@ -35,6 +35,7 @@
 #include <urcu/uatomic_arch.h>
 
 #include <ust/marker.h>
+#include <ust/tracepoint.h>
 #include <ust/tracectl.h>
 #include "tracer.h"
 #include "usterr.h"
@@ -127,6 +128,21 @@ static void print_markers(FILE *fp)
 	unlock_markers();
 }
 
+static void print_trace_events(FILE *fp)
+{
+	struct trace_event_iter iter;
+
+	lock_trace_events();
+	trace_event_iter_reset(&iter);
+	trace_event_iter_start(&iter);
+
+	while(iter.trace_event) {
+		fprintf(fp, "trace_event: %s\n", iter.trace_event->name);
+		trace_event_iter_next(&iter);
+	}
+	unlock_trace_events();
+}
+
 static int init_socket(void);
 
 /* Ask the daemon to collect a trace called trace_name and being
@@ -867,8 +883,22 @@ int process_client_cmd(char *recvbuf, struct ustcomm_source *src)
 		result = ustcomm_send_reply(&ustcomm_app.server, ptr, src);
 
 		free(ptr);
-	}
-	else if(!strcmp(recvbuf, "start")) {
+	} else if (!strcmp(recvbuf, "print_trace_events")) {
+		print_trace_events(stderr);
+
+	} else if(!strcmp(recvbuf, "list_trace_events")) {
+		char *ptr;
+		size_t size;
+		FILE *fp;
+
+		fp = open_memstream(&ptr, &size);
+		print_trace_events(fp);
+		fclose(fp);
+
+		result = ustcomm_send_reply(&ustcomm_app.server, ptr, src);
+
+		free(ptr);
+	} else if(!strcmp(recvbuf, "start")) {
 		/* start is an operation that setups the trace, allocates it and starts it */
 		result = ltt_trace_setup(trace_name);
 		if(result < 0) {
diff --git a/libustcmd/ustcmd.c b/libustcmd/ustcmd.c
index 5b4fd02..21e4846 100644
--- a/libustcmd/ustcmd.c
+++ b/libustcmd/ustcmd.c
@@ -440,6 +440,84 @@ int ustcmd_get_cmsf(struct marker_status **cmsf, const pid_t pid)
 	return 0;
 }
 
+
+/**
+ * Frees a TES array.
+ *
+ * @param tes	TES array to free
+ * @return	0 if successful, or error USTCMD_ERR_ARG
+ */
+int ustcmd_free_tes(struct trace_event_status *tes)
+{
+	if (tes == NULL) {
+		return USTCMD_ERR_ARG;
+	}
+
+	unsigned int i = 0;
+	while (tes[i].name != NULL) {
+		free(tes[i].name);
+		++i;
+	}
+	free(tes);
+
+	return 0;
+}
+
+/**
+ * Gets trace_events string for a given PID.
+ *
+ * @param tes	Pointer to TES array to be filled (callee allocates, caller
+ *		frees with `ustcmd_free_tes')
+ * @param pid	Targeted PID
+ * @return	0 if successful, or -1 on error
+ */
+int ustcmd_get_tes(struct trace_event_status **tes,
+			    const pid_t pid)
+{
+	char *big_str = NULL;
+	int result;
+	struct trace_event_status *tmp_tes = NULL;
+	unsigned int i = 0, tes_ind = 0;
+
+	if (tes == NULL) {
+		return -1;
+	}
+	result = ustcmd_send_cmd("list_trace_events", pid, &big_str);
+	if (result != 1) {
+		return -1;
+	}
+
+	if (result != 1) {
+		ERR("error while getting trace_event list");
+		return -1;
+	}
+
+	tmp_tes = (struct trace_event_status *) malloc(sizeof(struct trace_event_status) *
+						       (ustcmd_count_nl(big_str) + 1));
+	if (tmp_tes == NULL) {
+		return -1;
+	}
+
+	/* Parse received reply string (format: "[chan]/[mark] [st] [fs]"): */
+	while (big_str[i] != '\0') {
+		char state;
+
+		sscanf(big_str + i, "trace_event: %a[^\n]",
+			&tmp_tes[tes_ind].name);
+		while (big_str[i] != '\n') {
+			++i; /* Go to next '\n' */
+		}
+		++i; /* Skip current pointed '\n' */
+		++tes_ind;
+	}
+	tmp_tes[tes_ind].name = NULL;
+
+	*tes = tmp_tes;
+
+	free(big_str);
+	return 0;
+}
+
 /**
  * Set socket path
  *
diff --git a/ustctl/ustctl.c b/ustctl/ustctl.c
index d290975..9daa043 100644
--- a/ustctl/ustctl.c
+++ b/ustctl/ustctl.c
@@ -32,6 +32,7 @@ enum command {
 	STOP_TRACE,
 	DESTROY_TRACE,
 	LIST_MARKERS,
+	LIST_TRACE_EVENTS,
 	ENABLE_MARKER,
 	DISABLE_MARKER,
 	GET_ONLINE_PIDS,
@@ -73,6 +74,7 @@ Commands:\n\
     --enable-marker \"CHANNEL/MARKER\"\tEnable a marker\n\
     --disable-marker \"CHANNEL/MARKER\"\tDisable a marker\n\
     --list-markers\t\t\tList the markers of the process, their\n\t\t\t\t\t  state and format string\n\
+    --list-trace-events\t\t\tList the trace-events of the process\n\
     --force-switch\t\t\tForce a subbuffer switch\n\
 \
 ");
@@ -94,6 +96,7 @@ int parse_opts_long(int argc, char **argv, struct ust_opts *opts)
 			{ "stop-trace", 0, 0, STOP_TRACE },
 			{ "destroy-trace", 0, 0, DESTROY_TRACE },
 			{ "list-markers", 0, 0, LIST_MARKERS },
+			{ "list-trace-events", 0, 0, LIST_TRACE_EVENTS},
 			{ "enable-marker", 1, 0, ENABLE_MARKER },
 			{ "disable-marker", 1, 0, DISABLE_MARKER },
 			{ "help", 0, 0, 'h' },
@@ -215,6 +218,8 @@ int main(int argc, char *argv[])
 
 	pidit = opts.pids;
 	struct marker_status *cmsf = NULL;
+	struct trace_event_status *tes = NULL;
+	unsigned int i = 0;
 
 	while(*pidit != -1) {
 		switch (opts.cmd) {
@@ -258,7 +263,6 @@ int main(int argc, char *argv[])
 						" PID %u\n", (unsigned int) *pidit);
 					break;
 				}
-				unsigned int i = 0;
 				while (cmsf[i].channel != NULL) {
 					printf("{PID: %u, channel/marker: %s/%s, "
 						"state: %u, fmt: %s}\n",
@@ -272,6 +276,23 @@ int main(int argc, char *argv[])
 				ustcmd_free_cmsf(cmsf);
 				break;
 
+			case LIST_TRACE_EVENTS:
+				tes = NULL;
+				if (ustcmd_get_tes(&tes, *pidit)) {
+					fprintf(stderr,
+						"error while trying to list trace_events for"
+						" PID %u\n", (unsigned int) *pidit);
+					break;
+				}
+				while (tes[i].name != NULL) {
+					printf("{PID: %u, trace_event: %s}\n",
+					       (unsigned int) *pidit,
+					       tes[i].name);
+					++i;
+				}
+				ustcmd_free_tes(tes);
+
+				break;
 			case ENABLE_MARKER:
 				if(opts.regex)
 					ustcmd_set_marker_state(opts.regex, 1, *pidit);
-- 
1.7.1





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [ltt-dev] [UST PATCH 3/3] Add functions and command line for listing trace_events
  2010-09-08 17:52 ` [ltt-dev] [UST PATCH 3/3] Add functions and command line for listing trace_events Nils Carlson
@ 2010-09-08 18:23   ` David Goulet
  0 siblings, 0 replies; 4+ messages in thread
From: David Goulet @ 2010-09-08 18:23 UTC (permalink / raw)


Comment below.

On 10-09-08 01:52 PM, Nils Carlson wrote:
>
> Signed-off-by: Nils Carlson<nils.carlson at ericsson.com>
> ---
>   include/ust/ustcmd.h |    6 ++++
>   libust/tracectl.c    |   34 ++++++++++++++++++++-
>   libustcmd/ustcmd.c   |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   ustctl/ustctl.c      |   23 ++++++++++++++-
>   4 files changed, 138 insertions(+), 3 deletions(-)
>
> diff --git a/include/ust/ustcmd.h b/include/ust/ustcmd.h
> index 60f5018..986ae61 100644
> --- a/include/ust/ustcmd.h
> +++ b/include/ust/ustcmd.h
> @@ -43,6 +43,10 @@ struct marker_status {
>   	char *fs; /* Format string (end of marker_status array if NULL) */
>   };
>
> +struct trace_event_status {
> +	char *name;
> +};
> +
>   extern pid_t *ustcmd_get_online_pids(void);
>   extern int ustcmd_set_marker_state(const char *, int, pid_t);
>   extern int ustcmd_set_subbuf_size(const char *, pid_t);
> @@ -59,6 +63,8 @@ extern int ustcmd_free_cmsf(struct marker_status *);
>   extern unsigned int ustcmd_count_nl(const char *);
>   extern int ustcmd_send_cmd(const char *, pid_t, char **);
>   extern int ustcmd_get_cmsf(struct marker_status **, pid_t);
> +extern int ustcmd_free_tes(struct trace_event_status *);
> +extern int ustcmd_get_tes(struct trace_event_status **, pid_t);
>   extern int ustcmd_set_sock_path(const char *, pid_t);
>   extern int ustcmd_get_sock_path(char **, pid_t);
>   extern int ustcmd_force_switch(pid_t);
> diff --git a/libust/tracectl.c b/libust/tracectl.c
> index e64b26f..7dbc572 100644
> --- a/libust/tracectl.c
> +++ b/libust/tracectl.c
> @@ -35,6 +35,7 @@
>   #include<urcu/uatomic_arch.h>
>
>   #include<ust/marker.h>
> +#include<ust/tracepoint.h>
>   #include<ust/tracectl.h>
>   #include "tracer.h"
>   #include "usterr.h"
> @@ -127,6 +128,21 @@ static void print_markers(FILE *fp)
>   	unlock_markers();
>   }
>
> +static void print_trace_events(FILE *fp)
> +{
> +	struct trace_event_iter iter;
> +
> +	lock_trace_events();
> +	trace_event_iter_reset(&iter);
> +	trace_event_iter_start(&iter);
> +
> +	while(iter.trace_event) {
> +		fprintf(fp, "trace_event: %s\n", iter.trace_event->name);
> +		trace_event_iter_next(&iter);
> +	}
> +	unlock_trace_events();
> +}
> +
>   static int init_socket(void);
>
>   /* Ask the daemon to collect a trace called trace_name and being
> @@ -867,8 +883,22 @@ int process_client_cmd(char *recvbuf, struct ustcomm_source *src)
>   		result = ustcomm_send_reply(&ustcomm_app.server, ptr, src);
>
>   		free(ptr);
> -	}
> -	else if(!strcmp(recvbuf, "start")) {
> +	} else if (!strcmp(recvbuf, "print_trace_events")) {
> +		print_trace_events(stderr);
> +
> +	} else if(!strcmp(recvbuf, "list_trace_events")) {
> +		char *ptr;
> +		size_t size;
> +		FILE *fp;
> +
> +		fp = open_memstream(&ptr,&size);

Might want to check if fp is not NULL since print_trace_events don't 
check it either.

> +		print_trace_events(fp);
> +		fclose(fp);
> +
> +		result = ustcomm_send_reply(&ustcomm_app.server, ptr, src);
> +

Following the discussion on ltt-dev about error handling and trying to 
have consistency through the code, it might be good to handle the return 
code here and print the appropriate error. (ERR macro)

> +		free(ptr);
> +	} else if(!strcmp(recvbuf, "start")) {
>   		/* start is an operation that setups the trace, allocates it and starts it */
>   		result = ltt_trace_setup(trace_name);
>   		if(result<  0) {
> diff --git a/libustcmd/ustcmd.c b/libustcmd/ustcmd.c
> index 5b4fd02..21e4846 100644
> --- a/libustcmd/ustcmd.c
> +++ b/libustcmd/ustcmd.c
> @@ -440,6 +440,84 @@ int ustcmd_get_cmsf(struct marker_status **cmsf, const pid_t pid)
>   	return 0;
>   }
>
> +
> +/**
> + * Frees a TES array.
> + *
> + * @param tes	TES array to free
> + * @return	0 if successful, or error USTCMD_ERR_ARG
> + */
> +int ustcmd_free_tes(struct trace_event_status *tes)
> +{
> +	if (tes == NULL) {
> +		return USTCMD_ERR_ARG;
> +	}
> +
> +	unsigned int i = 0;
> +	while (tes[i].name != NULL) {
> +		free(tes[i].name);
> +		++i;
> +	}
> +	free(tes);
> +
> +	return 0;
> +}
> +
> +/**
> + * Gets trace_events string for a given PID.
> + *
> + * @param tes	Pointer to TES array to be filled (callee allocates, caller
> + *		frees with `ustcmd_free_tes')
> + * @param pid	Targeted PID
> + * @return	0 if successful, or -1 on error
> + */
> +int ustcmd_get_tes(struct trace_event_status **tes,
> +			    const pid_t pid)
> +{
> +	char *big_str = NULL;
> +	int result;
> +	struct trace_event_status *tmp_tes = NULL;
> +	unsigned int i = 0, tes_ind = 0;
> +
> +	if (tes == NULL) {
> +		return -1;
> +	}
> +	result = ustcmd_send_cmd("list_trace_events", pid,&big_str);

For a matter of code beauty :), there is a space missing after "pid, "

> +	if (result != 1) {
> +		return -1;
> +	}
> +
> +	if (result != 1) {
> +		ERR("error while getting trace_event list");
> +		return -1;
> +	}

This last if, isn't a bit useless ;)

> +
> +	tmp_tes = (struct trace_event_status *) malloc(sizeof(struct trace_event_status) *
> +						       (ustcmd_count_nl(big_str) + 1));

malloc -> zmalloc

> +	if (tmp_tes == NULL) {
> +		return -1;
> +	}
> +
> +	/* Parse received reply string (format: "[chan]/[mark] [st] [fs]"): */
> +	while (big_str[i] != '\0') {
> +		char state;
> +
> +		sscanf(big_str + i, "trace_event: %a[^\n]",
> +			&tmp_tes[tes_ind].name);
> +		while (big_str[i] != '\n') {
> +			++i; /* Go to next '\n' */
> +		}
> +		++i; /* Skip current pointed '\n' */
> +		++tes_ind;
> +	}
> +	tmp_tes[tes_ind].name = NULL;
> +
> +	*tes = tmp_tes;
> +
> +	free(big_str);
> +	return 0;
> +}
> +
>   /**
>    * Set socket path
>    *
> diff --git a/ustctl/ustctl.c b/ustctl/ustctl.c
> index d290975..9daa043 100644
> --- a/ustctl/ustctl.c
> +++ b/ustctl/ustctl.c
> @@ -32,6 +32,7 @@ enum command {
>   	STOP_TRACE,
>   	DESTROY_TRACE,
>   	LIST_MARKERS,
> +	LIST_TRACE_EVENTS,
>   	ENABLE_MARKER,
>   	DISABLE_MARKER,
>   	GET_ONLINE_PIDS,
> @@ -73,6 +74,7 @@ Commands:\n\
>       --enable-marker \"CHANNEL/MARKER\"\tEnable a marker\n\
>       --disable-marker \"CHANNEL/MARKER\"\tDisable a marker\n\
>       --list-markers\t\t\tList the markers of the process, their\n\t\t\t\t\t  state and format string\n\
> +    --list-trace-events\t\t\tList the trace-events of the process\n\
>       --force-switch\t\t\tForce a subbuffer switch\n\
>   \
>   ");
> @@ -94,6 +96,7 @@ int parse_opts_long(int argc, char **argv, struct ust_opts *opts)
>   			{ "stop-trace", 0, 0, STOP_TRACE },
>   			{ "destroy-trace", 0, 0, DESTROY_TRACE },
>   			{ "list-markers", 0, 0, LIST_MARKERS },
> +			{ "list-trace-events", 0, 0, LIST_TRACE_EVENTS},
>   			{ "enable-marker", 1, 0, ENABLE_MARKER },
>   			{ "disable-marker", 1, 0, DISABLE_MARKER },
>   			{ "help", 0, 0, 'h' },
> @@ -215,6 +218,8 @@ int main(int argc, char *argv[])
>
>   	pidit = opts.pids;
>   	struct marker_status *cmsf = NULL;
> +	struct trace_event_status *tes = NULL;
> +	unsigned int i = 0;
>
>   	while(*pidit != -1) {
>   		switch (opts.cmd) {
> @@ -258,7 +263,6 @@ int main(int argc, char *argv[])
>   						" PID %u\n", (unsigned int) *pidit);
>   					break;
>   				}
> -				unsigned int i = 0;
>   				while (cmsf[i].channel != NULL) {
>   					printf("{PID: %u, channel/marker: %s/%s, "
>   						"state: %u, fmt: %s}\n",
> @@ -272,6 +276,23 @@ int main(int argc, char *argv[])
>   				ustcmd_free_cmsf(cmsf);
>   				break;
>
> +			case LIST_TRACE_EVENTS:
> +				tes = NULL;

Isn't set to NULL before. Why again?

> +				if (ustcmd_get_tes(&tes, *pidit)) {
> +					fprintf(stderr,
> +						"error while trying to list trace_events for"
> +						" PID %u\n", (unsigned int) *pidit);

ERR macro do just that. We might want to use it everywhere instead of 
partial fprintf and ERR usage.

Thanks
David

> +					break;
> +				}
> +				while (tes[i].name != NULL) {
> +					printf("{PID: %u, trace_event: %s}\n",
> +					       (unsigned int) *pidit,
> +					       tes[i].name);
> +					++i;
> +				}
> +				ustcmd_free_tes(tes);
> +
> +				break;
>   			case ENABLE_MARKER:
>   				if(opts.regex)
>   					ustcmd_set_marker_state(opts.regex, 1, *pidit);

-- 
David Goulet
LTTng project, DORSAL Lab.

PGP/GPG : 1024D/16BD8563
BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-09-08 18:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-08 17:52 [ltt-dev] [UST PATCH 1/3] Add trace_event structs Nils Carlson
2010-09-08 17:52 ` [ltt-dev] [UST PATCH 2/3] Add trace_event library initialisation Nils Carlson
2010-09-08 17:52 ` [ltt-dev] [UST PATCH 3/3] Add functions and command line for listing trace_events Nils Carlson
2010-09-08 18:23   ` David Goulet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox