* [PATCH v2] libtracefs: Add new API for open trace marker file
@ 2021-04-08 8:08 Tzvetomir Stoyanov (VMware)
2021-04-08 12:59 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2021-04-08 8:08 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel
Added new APIs for opening trace_marker file of given instance:
tracefs_marker_init()
tracefs_marker_write()
tracefs_marker_vprint()
tracefs_marker_print()
tracefs_marker_close()
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
Apply on top of "libtracefs: Update filtering functions"
https://lore.kernel.org/linux-trace-devel/20210407202126.1870994-1-rostedt@goodmis.org/
v2 changes:
- Added set of new APIs, instead of the previously proposed API for just
opening the file.
include/tracefs-local.h | 1 +
include/tracefs.h | 9 +++
src/Makefile | 1 +
src/tracefs-instance.c | 3 +
src/tracefs-marker.c | 129 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 143 insertions(+)
create mode 100644 src/tracefs-marker.c
diff --git a/include/tracefs-local.h b/include/tracefs-local.h
index 6865611..6657b6f 100644
--- a/include/tracefs-local.h
+++ b/include/tracefs-local.h
@@ -21,6 +21,7 @@ struct tracefs_instance {
int flags;
int ftrace_filter_fd;
int ftrace_notrace_fd;
+ int ftrace_marker_fd;
};
extern pthread_mutex_t toplevel_lock;
diff --git a/include/tracefs.h b/include/tracefs.h
index 70b7ebe..9adcf07 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -63,6 +63,15 @@ static inline int tracefs_trace_on_get_fd(struct tracefs_instance *instance)
return tracefs_instance_file_open(instance, "tracing_on", O_RDWR);
}
+/* trace marker */
+int tracefs_marker_init(struct tracefs_instance *instance);
+int tracefs_marker_print(struct tracefs_instance *instance,
+ const char *fmt, ...);
+int tracefs_marker_vprint(struct tracefs_instance *instance,
+ const char *fmt, va_list ap);
+int tracefs_marker_write(struct tracefs_instance *instance, void *data, int len);
+void tracefs_marker_close(struct tracefs_instance *instance);
+
/* events */
void tracefs_list_free(char **list);
char **tracefs_event_systems(const char *tracing_dir);
diff --git a/src/Makefile b/src/Makefile
index dabdbb4..b4cff07 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -7,6 +7,7 @@ OBJS += tracefs-utils.o
OBJS += tracefs-instance.o
OBJS += tracefs-events.o
OBJS += tracefs-tools.o
+OBJS += tracefs-marker.o
OBJS := $(OBJS:%.o=$(bdir)/%.o)
DEPS := $(OBJS:$(bdir)/%.o=$(bdir)/.%.d)
diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c
index 599c3a7..90e35b4 100644
--- a/src/tracefs-instance.c
+++ b/src/tracefs-instance.c
@@ -49,6 +49,7 @@ static struct tracefs_instance *instance_alloc(const char *trace_dir, const char
instance->ftrace_filter_fd = -1;
instance->ftrace_notrace_fd = -1;
+ instance->ftrace_marker_fd = -1;
return instance;
@@ -74,6 +75,8 @@ void tracefs_instance_free(struct tracefs_instance *instance)
free(instance->trace_dir);
free(instance->name);
pthread_mutex_destroy(&instance->lock);
+ if (instance->ftrace_marker_fd >= 0)
+ close(instance->ftrace_marker_fd);
free(instance);
}
diff --git a/src/tracefs-marker.c b/src/tracefs-marker.c
new file mode 100644
index 0000000..92d55f8
--- /dev/null
+++ b/src/tracefs-marker.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2021, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+ *
+ */
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+
+#include "tracefs.h"
+#include "tracefs-local.h"
+
+/* File descriptor for Top level trace_marker */
+static int ftrace_marker_fd = -1;
+
+/**
+ * tracefs_marker_init - Open trace_marker file of selected instance for writing
+ * @instance: ftrace instance, can be NULL for top tracing instance.
+ *
+ * Returns 0 if the trace_marker is opened successfully, or -1 in case of an error
+ */
+int tracefs_marker_init(struct tracefs_instance *instance)
+{
+ int *fd = instance ? &instance->ftrace_marker_fd : &ftrace_marker_fd;
+
+ if (*fd >= 0)
+ return 0;
+ *fd = tracefs_instance_file_open(instance, "trace_marker", O_WRONLY);
+
+ return *fd < 0 ? -1 : 0;
+}
+
+/**
+ * tracefs_marker_write - Write binary data in the trace marker
+ * @instance: ftrace instance, can be NULL for top tracing instance.
+ * @data: binary data, that is going to be written in the trace marker
+ * @len: length of the @data
+ *
+ * If the trace marker file of the desired instance is not open already,
+ * this API will open it for writing. The file will stay open until
+ * tracefs_marker_close() is called.
+ *
+ * Returns 0 if the data is written correctly, or -1 in case of an error
+ */
+int tracefs_marker_write(struct tracefs_instance *instance, void *data, int len)
+{
+ int *fd = instance ? &instance->ftrace_marker_fd : &ftrace_marker_fd;
+ int ret;
+
+ if (!data || len < 1)
+ return -1;
+ if (*fd < 0) {
+ ret = tracefs_marker_init(instance);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = write(*fd, data, len);
+
+ return ret == len ? 0 : -1;
+}
+
+/**
+ * tracefs_marker_vprint - Write a formatted string in the trace marker
+ * @instance: ftrace instance, can be NULL for top tracing instance.
+ * @fmt: pritnf formatted string
+ * @ap: list of arguments for the formatted string
+ *
+ * If the trace marker file of the desired instance is not open already,
+ * this API will open it for writing. The file will stay open until
+ * tracefs_marker_close() is called.
+ *
+ * Returns 0 if the string is written correctly, or -1 in case of an error
+ */
+int tracefs_marker_vprint(struct tracefs_instance *instance,
+ const char *fmt, va_list ap)
+{
+ char *str = NULL;
+ int ret;
+
+ ret = vasprintf(&str, fmt, ap);
+ if (ret < 0)
+ return ret;
+ ret = tracefs_marker_write(instance, str, strlen(str) + 1);
+ free(str);
+
+ return ret;
+}
+
+/**
+ * tracefs_marker_print - Write a formatted string in the trace marker
+ * @instance: ftrace instance, can be NULL for top tracing instance.
+ * @fmt: pritnf formatted string with variable arguments ...
+ *
+ * If the trace marker file of the desired instance is not open already,
+ * this API will open it for writing. The file will stay open until
+ * tracefs_marker_close() is called.
+ *
+ * Returns 0 if the string is written correctly, or -1 in case of an error
+ */
+int tracefs_marker_print(struct tracefs_instance *instance,
+ const char *fmt, ...)
+{
+ va_list ap;
+ int ret;
+
+ va_start(ap, fmt);
+ ret = tracefs_marker_vprint(instance, fmt, ap);
+ va_end(ap);
+
+ return ret;
+}
+
+/**
+ * tracefs_marker_close - Close trace_marker file of selected instance
+ * @instance: ftrace instance, can be NULL for top tracing instance.
+ *
+ * Closes the file, previously opened with any of the other tracefs_marker_ APIs
+ */
+void tracefs_marker_close(struct tracefs_instance *instance)
+{
+ int *fd = instance ? &instance->ftrace_marker_fd : &ftrace_marker_fd;
+
+ if (*fd < 0)
+ return;
+ close(*fd);
+ *fd = -1;
+}
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] libtracefs: Add new API for open trace marker file
2021-04-08 8:08 [PATCH v2] libtracefs: Add new API for open trace marker file Tzvetomir Stoyanov (VMware)
@ 2021-04-08 12:59 ` Steven Rostedt
2021-04-08 13:21 ` Tzvetomir Stoyanov
0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2021-04-08 12:59 UTC (permalink / raw)
To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel
On Thu, 8 Apr 2021 11:08:21 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
> Added new APIs for opening trace_marker file of given instance:
> tracefs_marker_init()
> tracefs_marker_write()
> tracefs_marker_vprint()
> tracefs_marker_print()
> tracefs_marker_close()
I really don't like the name as it's more about the tracefs implementation
and not what it's used for. I much rather have:
tracefs_print_init();
tracefs_printf();
tracefs_vprintf();
tracefs_print_close();
And you should never write binary into the trace_marker file, as it always
expects ascii strings. That's the trace_marker_raw file. In which case we
could have:
tracefs_binary_init();
tracefs_binary_write();
tracefs_binary_close();
For binary writes.
-- Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] libtracefs: Add new API for open trace marker file
2021-04-08 12:59 ` Steven Rostedt
@ 2021-04-08 13:21 ` Tzvetomir Stoyanov
2021-04-08 13:41 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Tzvetomir Stoyanov @ 2021-04-08 13:21 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Linux Trace Devel
On Thu, Apr 8, 2021 at 3:59 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 8 Apr 2021 11:08:21 +0300
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > Added new APIs for opening trace_marker file of given instance:
> > tracefs_marker_init()
> > tracefs_marker_write()
> > tracefs_marker_vprint()
> > tracefs_marker_print()
> > tracefs_marker_close()
>
> I really don't like the name as it's more about the tracefs implementation
> and not what it's used for. I much rather have:
>
> tracefs_print_init();
> tracefs_printf();
> tracefs_vprintf();
> tracefs_print_close();
>
> And you should never write binary into the trace_marker file, as it always
> expects ascii strings. That's the trace_marker_raw file. In which case we
> could have:
>
> tracefs_binary_init();
> tracefs_binary_write();
> tracefs_binary_close();
>
> For binary writes.
>
I find "tracefs_print_" a little bit confusing, like printing
something on console. I think the name should stress that a user
string/data is written in the trace buffer. I would like to combine
string and binary APIs into one set, something like:
tracefs_user_trace_init(); /* open both trace_marker and
trace_marker_raw files, or have flags to specify what file to open,
string or data*/
tracefs_user_trace_printf(); /* write to trace_marker */
tracefs_user_trace_vprintf(); /* write to trace_marker */
tracefs_user_trace_binary(); /* write to trace_marker_raw */
tracefs_user_trace_init(); /* close both string and data fd, if open */
> -- Steve
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] libtracefs: Add new API for open trace marker file
2021-04-08 13:21 ` Tzvetomir Stoyanov
@ 2021-04-08 13:41 ` Steven Rostedt
0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2021-04-08 13:41 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: Linux Trace Devel
On Thu, 8 Apr 2021 16:21:52 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
> I find "tracefs_print_" a little bit confusing, like printing
> something on console. I think the name should stress that a user
> string/data is written in the trace buffer. I would like to combine
> string and binary APIs into one set, something like:
How so? We use it in the kernel "trace_printk()" there's no confusion there.
And to me, "user string written in the trace buffer" is exactly what
I think when I see "tracefs_printf()".
>
> tracefs_user_trace_init(); /* open both trace_marker and
> trace_marker_raw files, or have flags to specify what file to open,
> string or data*/
> tracefs_user_trace_printf(); /* write to trace_marker */
> tracefs_user_trace_vprintf(); /* write to trace_marker */
> tracefs_user_trace_binary(); /* write to trace_marker_raw */
> tracefs_user_trace_init(); /* close both string and data fd, if open */
>
I have no idea what "user_trace" is. What does that mean? There's no
precedence for it.
"tracefs_printf()" is short and to the point, and states exactly what you
want. It does a printf() into the tracefs system. How is that confusing?
We have fprintf() which does a printf into a file point.
We have sprintf() which does an printf into a string.
Thus, it makes sense to have "tracefs_printf()" which does a printf into
the tracefs file system. Just like we have trace_seq_printf() which does a
printf() into the trace_seq.
"printf()" means string manipulation, and has nothing to do with consoles.
I personally hate long function names, especially ones that I may use
a lot of.
Keeping with the other APIs trace_printf() is the only one that seems
reasonable.
-- Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-08 13:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-08 8:08 [PATCH v2] libtracefs: Add new API for open trace marker file Tzvetomir Stoyanov (VMware)
2021-04-08 12:59 ` Steven Rostedt
2021-04-08 13:21 ` Tzvetomir Stoyanov
2021-04-08 13:41 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).