* [PATCH 0/2] Enhancements to trace-cmd clear @ 2020-07-17 8:03 Tzvetomir Stoyanov (VMware) 2020-07-17 8:03 ` [PATCH 1/2] trace-cmd: Add new tracefs API tracefs_instances_walk() Tzvetomir Stoyanov (VMware) 2020-07-17 8:03 ` [PATCH 2/2] trace-cmd: Enhance "trace-cmd clear" to be instance aware Tzvetomir Stoyanov (VMware) 0 siblings, 2 replies; 7+ messages in thread From: Tzvetomir Stoyanov (VMware) @ 2020-07-17 8:03 UTC (permalink / raw) To: rostedt; +Cc: linux-trace-devel Added new options to trace-cmd clear, alowwing it to be ftrace instance aware. Implemented new tracefs API for iterating over the configured ftrace instances. Tzvetomir Stoyanov (VMware) (2): trace-cmd: Add new tracefs API tracefs_instances_walk() trace-cmd: Enhance "trace-cmd clear" to be instance aware Documentation/trace-cmd-clear.1.txt | 13 ++- include/tracefs/tracefs.h | 1 + lib/tracefs/include/tracefs-local.h | 1 + lib/tracefs/tracefs-events.c | 14 ++-- lib/tracefs/tracefs-instance.c | 52 ++++++++++++ tracecmd/Makefile | 1 + tracecmd/trace-clear.c | 124 ++++++++++++++++++++++++++++ tracecmd/trace-record.c | 24 ------ tracecmd/trace-stat.c | 52 +++--------- tracecmd/trace-usage.c | 4 +- 10 files changed, 214 insertions(+), 72 deletions(-) create mode 100644 tracecmd/trace-clear.c -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] trace-cmd: Add new tracefs API tracefs_instances_walk() 2020-07-17 8:03 [PATCH 0/2] Enhancements to trace-cmd clear Tzvetomir Stoyanov (VMware) @ 2020-07-17 8:03 ` Tzvetomir Stoyanov (VMware) 2020-07-17 13:25 ` Steven Rostedt 2020-07-17 8:03 ` [PATCH 2/2] trace-cmd: Enhance "trace-cmd clear" to be instance aware Tzvetomir Stoyanov (VMware) 1 sibling, 1 reply; 7+ messages in thread From: Tzvetomir Stoyanov (VMware) @ 2020-07-17 8:03 UTC (permalink / raw) To: rostedt; +Cc: linux-trace-devel The logic for finding all configured ftrace instances is encapuslated in the new tracefs_instances_walk() API. A user specified callback is called for each ftrace instance in the system, excpet for the top level one. The implementation of "trace-cmd stat" is modified to use the new API. Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- include/tracefs/tracefs.h | 1 + lib/tracefs/include/tracefs-local.h | 1 + lib/tracefs/tracefs-events.c | 14 ++++---- lib/tracefs/tracefs-instance.c | 52 +++++++++++++++++++++++++++++ tracecmd/trace-stat.c | 52 ++++++++--------------------- 5 files changed, 74 insertions(+), 46 deletions(-) diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h index 8ee7ba6e..0dd8046f 100644 --- a/include/tracefs/tracefs.h +++ b/include/tracefs/tracefs.h @@ -32,6 +32,7 @@ int tracefs_instance_file_write(struct tracefs_instance *instance, const char *file, const char *str); char *tracefs_instance_file_read(struct tracefs_instance *instance, char *file, int *psize); +int tracefs_instances_walk(int (*callback)(const char *, void *), void *context); bool tracefs_file_exists(struct tracefs_instance *instance, char *name); bool tracefs_dir_exists(struct tracefs_instance *instance, char *name); diff --git a/lib/tracefs/include/tracefs-local.h b/lib/tracefs/include/tracefs-local.h index fe327a0f..08b67fa9 100644 --- a/lib/tracefs/include/tracefs-local.h +++ b/lib/tracefs/include/tracefs-local.h @@ -9,5 +9,6 @@ /* Can be overridden */ void warning(const char *fmt, ...); int str_read_file(const char *file, char **buffer); +char *trace_append_file(const char *dir, const char *name); #endif /* _TRACE_FS_LOCAL_H */ diff --git a/lib/tracefs/tracefs-events.c b/lib/tracefs/tracefs-events.c index 8e825f50..564a5ab2 100644 --- a/lib/tracefs/tracefs-events.c +++ b/lib/tracefs/tracefs-events.c @@ -210,7 +210,7 @@ static char **add_list_string(char **list, const char *name, int len) return list; } -static char *append_file(const char *dir, const char *name) +char *trace_append_file(const char *dir, const char *name) { char *file; int ret; @@ -265,7 +265,7 @@ char **tracefs_event_systems(const char *tracing_dir) if (!tracing_dir) return NULL; - events_dir = append_file(tracing_dir, "events"); + events_dir = trace_append_file(tracing_dir, "events"); if (!events_dir) return NULL; @@ -290,14 +290,14 @@ char **tracefs_event_systems(const char *tracing_dir) strcmp(name, "..") == 0) continue; - sys = append_file(events_dir, name); + sys = trace_append_file(events_dir, name); ret = stat(sys, &st); if (ret < 0 || !S_ISDIR(st.st_mode)) { free(sys); continue; } - enable = append_file(sys, "enable"); + enable = trace_append_file(sys, "enable"); ret = stat(enable, &st); if (ret >= 0) @@ -359,7 +359,7 @@ char **tracefs_system_events(const char *tracing_dir, const char *system) strcmp(name, "..") == 0) continue; - event = append_file(system_dir, name); + event = trace_append_file(system_dir, name); ret = stat(event, &st); if (ret < 0 || !S_ISDIR(st.st_mode)) { free(event); @@ -401,7 +401,7 @@ char **tracefs_tracers(const char *tracing_dir) if (!tracing_dir) return NULL; - available_tracers = append_file(tracing_dir, "available_tracers"); + available_tracers = trace_append_file(tracing_dir, "available_tracers"); if (!available_tracers) return NULL; @@ -497,7 +497,7 @@ static int read_header(struct tep_handle *tep, const char *tracing_dir) int len; int ret = -1; - header = append_file(tracing_dir, "events/header_page"); + header = trace_append_file(tracing_dir, "events/header_page"); ret = stat(header, &st); if (ret < 0) diff --git a/lib/tracefs/tracefs-instance.c b/lib/tracefs/tracefs-instance.c index 50e88534..befc0333 100644 --- a/lib/tracefs/tracefs-instance.c +++ b/lib/tracefs/tracefs-instance.c @@ -13,6 +13,7 @@ #include <errno.h> #include <sys/stat.h> #include <fcntl.h> +#include <dirent.h> #include <linux/limits.h> #include "tracefs.h" #include "tracefs-local.h" @@ -291,3 +292,54 @@ bool tracefs_dir_exists(struct tracefs_instance *instance, char *name) { return check_file_exists(instance, name, true); } + +/** + * tracefs_instances_walk - Iterate through all ftrace instances in the system + * @callback: user callback, called for each instance. Instance name is passed + * as input parameter. If the @callback returns non-zero, + * the iteration stops. + * @context: user context, passed to the @callback. + * + * Returns -1 in case of an error, 0 otherwise. + */ +int tracefs_instances_walk(int (*callback)(const char *, void *), void *context) +{ + struct dirent *dent; + char *path = NULL; + DIR *dir = NULL; + struct stat st; + int fret = -1; + int ret; + + path = tracefs_get_tracing_file("instances"); + if (!path) + return -1; + ret = stat(path, &st); + if (ret < 0 || !S_ISDIR(st.st_mode)) + goto out; + + dir = opendir(path); + if (!dir) + goto out; + fret = 0; + while ((dent = readdir(dir))) { + char *instance; + + if (strcmp(dent->d_name, ".") == 0 || + strcmp(dent->d_name, "..") == 0) + continue; + instance = trace_append_file(path, dent->d_name); + ret = stat(instance, &st); + free(instance); + if (ret < 0 || !S_ISDIR(st.st_mode)) + continue; + if (callback(dent->d_name, context)) + break; + } + +out: + if (dir) + closedir(dir); + tracefs_put_tracing_file(path); + return fret; +} diff --git a/tracecmd/trace-stat.c b/tracecmd/trace-stat.c index 260d0c37..c242dae2 100644 --- a/tracecmd/trace-stat.c +++ b/tracecmd/trace-stat.c @@ -7,7 +7,6 @@ #include <sys/stat.h> #include <stdlib.h> #include <stdio.h> -#include <dirent.h> #include <getopt.h> #include <unistd.h> #include <fcntl.h> @@ -140,48 +139,23 @@ static void report_file(struct buffer_instance *instance, free(str); } -static void report_instances(void) +static int report_instance(const char *name, void *data) { - struct dirent *dent; - bool first = true; - char *path = NULL; - DIR *dir = NULL; - struct stat st; - int ret; - - path = tracefs_get_tracing_file("instances"); - if (!path) - return; - ret = stat(path, &st); - if (ret < 0 || !S_ISDIR(st.st_mode)) - goto out; - - dir = opendir(path); - if (!dir) - goto out; - - while ((dent = readdir(dir))) { - char *instance; + bool *first = (bool *)data; - if (strcmp(dent->d_name, ".") == 0 || - strcmp(dent->d_name, "..") == 0) - continue; - instance = append_file(path, dent->d_name); - ret = stat(instance, &st); - free(instance); - if (ret < 0 || !S_ISDIR(st.st_mode)) - continue; - if (first) { - first = false; - printf("\nInstances:\n"); - } - printf(" %s\n", dent->d_name); + if (*first) { + *first = false; + printf("\nInstances:\n"); } + printf(" %s\n", name); + return 0; +} -out: - if (dir) - closedir(dir); - tracefs_put_tracing_file(path); +static void report_instances(void) +{ + bool first = true; + + tracefs_instances_walk(report_instance, &first); } struct event_iter *trace_event_iter_alloc(const char *path) -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] trace-cmd: Add new tracefs API tracefs_instances_walk() 2020-07-17 8:03 ` [PATCH 1/2] trace-cmd: Add new tracefs API tracefs_instances_walk() Tzvetomir Stoyanov (VMware) @ 2020-07-17 13:25 ` Steven Rostedt 0 siblings, 0 replies; 7+ messages in thread From: Steven Rostedt @ 2020-07-17 13:25 UTC (permalink / raw) To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel On Fri, 17 Jul 2020 11:03:25 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > The logic for finding all configured ftrace instances is encapuslated in > the new tracefs_instances_walk() API. A user specified callback is > called for each ftrace instance in the system, excpet for the top level > one. > The implementation of "trace-cmd stat" is modified to use the new API. > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > include/tracefs/tracefs.h | 1 + > lib/tracefs/include/tracefs-local.h | 1 + > lib/tracefs/tracefs-events.c | 14 ++++---- > lib/tracefs/tracefs-instance.c | 52 +++++++++++++++++++++++++++++ > tracecmd/trace-stat.c | 52 ++++++++--------------------- > 5 files changed, 74 insertions(+), 46 deletions(-) > > diff --git a/include/tracefs/tracefs.h b/include/tracefs/tracefs.h > index 8ee7ba6e..0dd8046f 100644 > --- a/include/tracefs/tracefs.h > +++ b/include/tracefs/tracefs.h > @@ -32,6 +32,7 @@ int tracefs_instance_file_write(struct tracefs_instance *instance, > const char *file, const char *str); > char *tracefs_instance_file_read(struct tracefs_instance *instance, > char *file, int *psize); > +int tracefs_instances_walk(int (*callback)(const char *, void *), void *context); Perhaps a better name would be: tracefs_instance_iterate()? I usually consider a "walk" as going through a tree, not a list. If this was a directory tree, then "walk" would be more appropriate. An "iterator" is something that walks through a list and triggers a callback, like this does. But other than that, the patch looks good. -- Steve > > bool tracefs_file_exists(struct tracefs_instance *instance, char *name); > bool tracefs_dir_exists(struct tracefs_instance *instance, char *name); > diff --git a/lib/tracefs/include/tracefs-local.h b/lib/tracefs/include/tracefs-local.h > index fe327a0f..08b67fa9 100644 > --- a/lib/tracefs/include/tracefs-local.h > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] trace-cmd: Enhance "trace-cmd clear" to be instance aware 2020-07-17 8:03 [PATCH 0/2] Enhancements to trace-cmd clear Tzvetomir Stoyanov (VMware) 2020-07-17 8:03 ` [PATCH 1/2] trace-cmd: Add new tracefs API tracefs_instances_walk() Tzvetomir Stoyanov (VMware) @ 2020-07-17 8:03 ` Tzvetomir Stoyanov (VMware) 2020-07-17 13:31 ` Steven Rostedt 2020-07-17 14:06 ` Steven Rostedt 1 sibling, 2 replies; 7+ messages in thread From: Tzvetomir Stoyanov (VMware) @ 2020-07-17 8:03 UTC (permalink / raw) To: rostedt; +Cc: linux-trace-devel Added new options "trace-cmd clear -B <name> -a" which allow the command to work per ftrace instance: -B clear the given buffer (may specify multiple -B) -a clear all existing buffers, including the top level one Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- Documentation/trace-cmd-clear.1.txt | 13 ++- tracecmd/Makefile | 1 + tracecmd/trace-clear.c | 124 ++++++++++++++++++++++++++++ tracecmd/trace-record.c | 24 ------ tracecmd/trace-usage.c | 4 +- 5 files changed, 140 insertions(+), 26 deletions(-) create mode 100644 tracecmd/trace-clear.c diff --git a/Documentation/trace-cmd-clear.1.txt b/Documentation/trace-cmd-clear.1.txt index 67e5851a..a0ae36e9 100644 --- a/Documentation/trace-cmd-clear.1.txt +++ b/Documentation/trace-cmd-clear.1.txt @@ -7,12 +7,23 @@ trace-cmd-clear - clear the Ftrace buffer. SYNOPSIS -------- -*trace-cmd clear* +*trace-cmd clear* ['OPTIONS'] DESCRIPTION ----------- The *trace-cmd(1) clear* clears the content of the Ftrace ring buffer. +OPTIONS +------- +*-B* 'buffer-name':: + If the kernel supports multiple buffers, this will clear only the given + buffer. It does not affect any other buffers. This may be used multiple + times to specify different buffers. The top level buffer will not be + clearded if this option is given. + +*-a*:: + Clear all existing buffers, including the top level one. + SEE ALSO -------- trace-cmd(1), trace-cmd-record(1), trace-cmd-report(1), trace-cmd-start(1), diff --git a/tracecmd/Makefile b/tracecmd/Makefile index 5e59adf8..01f36c61 100644 --- a/tracecmd/Makefile +++ b/tracecmd/Makefile @@ -31,6 +31,7 @@ TRACE_CMD_OBJS += trace-show.o TRACE_CMD_OBJS += trace-list.o TRACE_CMD_OBJS += trace-usage.o TRACE_CMD_OBJS += trace-dump.o +TRACE_CMD_OBJS += trace-clear.o ifeq ($(VSOCK_DEFINED), 1) TRACE_CMD_OBJS += trace-tsync.o endif diff --git a/tracecmd/trace-clear.c b/tracecmd/trace-clear.c new file mode 100644 index 00000000..f1b4f309 --- /dev/null +++ b/tracecmd/trace-clear.c @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: LGPL-2.1 +/* + * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> + * + * Updates: + * Copyright (C) 2020, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com> + * + */ +#include <stdlib.h> +#include <unistd.h> +#include <getopt.h> + +#include "tracefs.h" +#include "trace-local.h" + +struct instances_list { + struct instances_list *next; + struct tracefs_instance *instance; +}; + +static int add_new_instance(struct instances_list **list, char *name) +{ + struct instances_list *new; + + new = calloc(1, sizeof(*new)); + if (!new) + return -1; + new->instance = tracefs_instance_alloc(name); + if (!new->instance) { + free(new); + return -1; + } + + new->next = *list; + *list = new; + return 0; +} + +static int add_instance_walk(const char *name, void *data) +{ + return add_new_instance((struct instances_list **)data, (char *)name); +} + +static void clear_list(struct instances_list *list) +{ + struct instances_list *del; + + while (list) { + del = list; + list = list->next; + tracefs_instance_free(del->instance); + free(del); + } +} + +static void clear_instance_trace(struct tracefs_instance *instance) +{ + FILE *fp; + char *path; + + /* reset the trace */ + path = tracefs_instance_get_file(instance, "trace"); + fp = fopen(path, "w"); + if (!fp) + die("writing to '%s'", path); + tracefs_put_tracing_file(path); + fwrite("0", 1, 1, fp); + fclose(fp); +} + +static void clear_trace(struct instances_list *instances) +{ + if (instances) { + while (instances) { + clear_instance_trace(instances->instance); + instances = instances->next; + } + } else + clear_instance_trace(NULL); +} + +void trace_clear(int argc, char **argv) +{ + struct instances_list *instances = NULL; + bool all = false; + int c; + + for (;;) { + int option_index = 0; + static struct option long_options[] = { + {"all", no_argument, NULL, 'a'}, + {"help", no_argument, NULL, '?'}, + {NULL, 0, NULL, 0} + }; + + c = getopt_long (argc-1, argv+1, "+haB:", + long_options, &option_index); + if (c == -1) + break; + switch (c) { + case 'B': + if (add_new_instance(&instances, optarg)) + die("Failed to allocate instance %s", optarg); + break; + case 'a': + all = true; + if (tracefs_instances_walk(add_instance_walk, &instances)) + die("Failed to add all instances"); + break; + case 'h': + case '?': + default: + usage(argv); + break; + } + } + + clear_trace(instances); + if (all) + clear_trace(NULL); + clear_list(instances); + exit(0); +} + diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index bd004574..5009eaa1 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -835,21 +835,6 @@ static void clear_trace_instances(void) __clear_trace(instance); } -static void clear_trace(void) -{ - FILE *fp; - char *path; - - /* reset the trace */ - path = tracefs_get_tracing_file("trace"); - fp = fopen(path, "w"); - if (!fp) - die("writing to '%s'", path); - tracefs_put_tracing_file(path); - fwrite("0", 1, 1, fp); - fclose(fp); -} - static void reset_max_latency(struct buffer_instance *instance) { tracefs_instance_file_write(instance->tracefs, @@ -6704,15 +6689,6 @@ void trace_profile(int argc, char **argv) exit(0); } -void trace_clear(int argc, char **argv) -{ - if (argc > 2) - usage(argv); - else - clear_trace(); - exit(0); -} - void trace_record(int argc, char **argv) { struct common_record_context ctx; diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c index 3f0b2d07..79610bd3 100644 --- a/tracecmd/trace-usage.c +++ b/tracecmd/trace-usage.c @@ -180,7 +180,9 @@ static struct usage_help usage_help[] = { { "clear", "clear the trace buffers", - " %s clear\n" + " %s clear [-B buf][-a]\n" + " -B clear the given buffer (may specify multiple -B)\n" + " -a clear all existing buffers, including the top level one\n" }, { "report", -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] trace-cmd: Enhance "trace-cmd clear" to be instance aware 2020-07-17 8:03 ` [PATCH 2/2] trace-cmd: Enhance "trace-cmd clear" to be instance aware Tzvetomir Stoyanov (VMware) @ 2020-07-17 13:31 ` Steven Rostedt 2020-07-17 14:06 ` Steven Rostedt 1 sibling, 0 replies; 7+ messages in thread From: Steven Rostedt @ 2020-07-17 13:31 UTC (permalink / raw) To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel On Fri, 17 Jul 2020 11:03:26 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > Added new options "trace-cmd clear -B <name> -a" which allow the command > to work per ftrace instance: > -B clear the given buffer (may specify multiple -B) > -a clear all existing buffers, including the top level one > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > Documentation/trace-cmd-clear.1.txt | 13 ++- > tracecmd/Makefile | 1 + > tracecmd/trace-clear.c | 124 ++++++++++++++++++++++++++++ > tracecmd/trace-record.c | 24 ------ > tracecmd/trace-usage.c | 4 +- > 5 files changed, 140 insertions(+), 26 deletions(-) > create mode 100644 tracecmd/trace-clear.c > > diff --git a/Documentation/trace-cmd-clear.1.txt b/Documentation/trace-cmd-clear.1.txt > index 67e5851a..a0ae36e9 100644 > --- a/Documentation/trace-cmd-clear.1.txt > +++ b/Documentation/trace-cmd-clear.1.txt > @@ -7,12 +7,23 @@ trace-cmd-clear - clear the Ftrace buffer. > > SYNOPSIS > -------- > -*trace-cmd clear* > +*trace-cmd clear* ['OPTIONS'] > > DESCRIPTION > ----------- > The *trace-cmd(1) clear* clears the content of the Ftrace ring buffer. > > +OPTIONS > +------- > +*-B* 'buffer-name':: > + If the kernel supports multiple buffers, this will clear only the given > + buffer. It does not affect any other buffers. This may be used multiple > + times to specify different buffers. The top level buffer will not be > + clearded if this option is given. We should give a special name for the top buffer. I think "/" would be good, as that can not be in the name of any instance. That way if you have three buffers (top and two instances, say "foo" and "bar") we could do something like: trace-cmd clear -B / -B foo And it will clear the top buffer and "foo", but leave "bar" alone. > + > +*-a*:: > + Clear all existing buffers, including the top level one. > + > SEE ALSO > -------- > trace-cmd(1), trace-cmd-record(1), trace-cmd-report(1), trace-cmd-start(1), > diff --git a/tracecmd/Makefile b/tracecmd/Makefile > index 5e59adf8..01f36c61 100644 > --- a/tracecmd/Makefile > +++ b/tracecmd/Makefile > @@ -31,6 +31,7 @@ TRACE_CMD_OBJS += trace-show.o > TRACE_CMD_OBJS += trace-list.o > TRACE_CMD_OBJS += trace-usage.o > TRACE_CMD_OBJS += trace-dump.o > +TRACE_CMD_OBJS += trace-clear.o > ifeq ($(VSOCK_DEFINED), 1) > TRACE_CMD_OBJS += trace-tsync.o > endif > diff --git a/tracecmd/trace-clear.c b/tracecmd/trace-clear.c > new file mode 100644 > index 00000000..f1b4f309 > --- /dev/null > +++ b/tracecmd/trace-clear.c Thinking about this more, the functionality of this entire file should be in libtracefs. This is specific to the tracefs functionality and not to trace-cmd in general. We should have: tracefs_clear(instance_name); Where: tracefs_clear(NULL) clears the top level. tracefs_clear("foo") clears the instance named "foo". And a tracefs_clear_all(bool top); Where: tracefs_clear_all(false) clear's all instances but leaves the top instance untouched. tracefs_clear_all(true) clears the top level and all instances. -- Steve > @@ -0,0 +1,124 @@ > +// SPDX-License-Identifier: LGPL-2.1 > +/* > + * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> > + * > + * Updates: > + * Copyright (C) 2020, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com> > + * > + */ > +#include <stdlib.h> > +#include <unistd.h> > +#include <getopt.h> > + > +#include "tracefs.h" > +#include "trace-local.h" > + > +struct instances_list { > + struct instances_list *next; > + struct tracefs_instance *instance; > +}; > + > +static int add_new_instance(struct instances_list **list, char *name) > +{ > + struct instances_list *new; > + > + new = calloc(1, sizeof(*new)); > + if (!new) > + return -1; > + new->instance = tracefs_instance_alloc(name); > + if (!new->instance) { > + free(new); > + return -1; > + } > + > + new->next = *list; > + *list = new; > + return 0; > +} > + > +static int add_instance_walk(const char *name, void *data) > +{ > + return add_new_instance((struct instances_list **)data, (char *)name); > +} > + > +static void clear_list(struct instances_list *list) > +{ > + struct instances_list *del; > + > + while (list) { > + del = list; > + list = list->next; > + tracefs_instance_free(del->instance); > + free(del); > + } > +} > + > +static void clear_instance_trace(struct tracefs_instance *instance) > +{ > + FILE *fp; > + char *path; > + > + /* reset the trace */ > + path = tracefs_instance_get_file(instance, "trace"); > + fp = fopen(path, "w"); > + if (!fp) > + die("writing to '%s'", path); > + tracefs_put_tracing_file(path); > + fwrite("0", 1, 1, fp); > + fclose(fp); > +} > + > +static void clear_trace(struct instances_list *instances) > +{ > + if (instances) { > + while (instances) { > + clear_instance_trace(instances->instance); > + instances = instances->next; > + } > + } else > + clear_instance_trace(NULL); > +} > + > +void trace_clear(int argc, char **argv) > +{ > + struct instances_list *instances = NULL; > + bool all = false; > + int c; > + > + for (;;) { > + int option_index = 0; > + static struct option long_options[] = { > + {"all", no_argument, NULL, 'a'}, > + {"help", no_argument, NULL, '?'}, > + {NULL, 0, NULL, 0} > + }; > + > + c = getopt_long (argc-1, argv+1, "+haB:", > + long_options, &option_index); > + if (c == -1) > + break; > + switch (c) { > + case 'B': > + if (add_new_instance(&instances, optarg)) > + die("Failed to allocate instance %s", optarg); > + break; > + case 'a': > + all = true; > + if (tracefs_instances_walk(add_instance_walk, &instances)) > + die("Failed to add all instances"); > + break; > + case 'h': > + case '?': > + default: > + usage(argv); > + break; > + } > + } > + > + clear_trace(instances); > + if (all) > + clear_trace(NULL); > + clear_list(instances); > + exit(0); > +} > + > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index bd004574..5009eaa1 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -835,21 +835,6 @@ static void clear_trace_instances(void) > __clear_trace(instance); > } > > -static void clear_trace(void) > -{ > - FILE *fp; > - char *path; > - > - /* reset the trace */ > - path = tracefs_get_tracing_file("trace"); > - fp = fopen(path, "w"); > - if (!fp) > - die("writing to '%s'", path); > - tracefs_put_tracing_file(path); > - fwrite("0", 1, 1, fp); > - fclose(fp); > -} > - > static void reset_max_latency(struct buffer_instance *instance) > { > tracefs_instance_file_write(instance->tracefs, > @@ -6704,15 +6689,6 @@ void trace_profile(int argc, char **argv) > exit(0); > } > > -void trace_clear(int argc, char **argv) > -{ > - if (argc > 2) > - usage(argv); > - else > - clear_trace(); > - exit(0); > -} > - > void trace_record(int argc, char **argv) > { > struct common_record_context ctx; > diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c > index 3f0b2d07..79610bd3 100644 > --- a/tracecmd/trace-usage.c > +++ b/tracecmd/trace-usage.c > @@ -180,7 +180,9 @@ static struct usage_help usage_help[] = { > { > "clear", > "clear the trace buffers", > - " %s clear\n" > + " %s clear [-B buf][-a]\n" > + " -B clear the given buffer (may specify multiple -B)\n" > + " -a clear all existing buffers, including the top level one\n" > }, > { > "report", ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] trace-cmd: Enhance "trace-cmd clear" to be instance aware 2020-07-17 8:03 ` [PATCH 2/2] trace-cmd: Enhance "trace-cmd clear" to be instance aware Tzvetomir Stoyanov (VMware) 2020-07-17 13:31 ` Steven Rostedt @ 2020-07-17 14:06 ` Steven Rostedt 2020-07-17 14:29 ` Tzvetomir Stoyanov 1 sibling, 1 reply; 7+ messages in thread From: Steven Rostedt @ 2020-07-17 14:06 UTC (permalink / raw) To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel On Fri, 17 Jul 2020 11:03:26 +0300 "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > --- /dev/null > +++ b/tracecmd/trace-clear.c > @@ -0,0 +1,124 @@ > +// SPDX-License-Identifier: LGPL-2.1 > +/* > + * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> > + * > + * Updates: > + * Copyright (C) 2020, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com> > + * > + */ > +#include <stdlib.h> > +#include <unistd.h> > +#include <getopt.h> > + > +#include "tracefs.h" > +#include "trace-local.h" > + > +struct instances_list { > + struct instances_list *next; > + struct tracefs_instance *instance; > +}; > + > +static int add_new_instance(struct instances_list **list, char *name) > +{ > + struct instances_list *new; > + > + new = calloc(1, sizeof(*new)); > + if (!new) > + return -1; > + new->instance = tracefs_instance_alloc(name); > + if (!new->instance) { > + free(new); > + return -1; > + } > + > + new->next = *list; > + *list = new; > + return 0; > +} > + > +static int add_instance_walk(const char *name, void *data) > +{ > + return add_new_instance((struct instances_list **)data, (char *)name); > +} > + > +static void clear_list(struct instances_list *list) > +{ > + struct instances_list *del; > + > + while (list) { > + del = list; > + list = list->next; > + tracefs_instance_free(del->instance); > + free(del); > + } > +} BTW, why create this list? (see below). > + > +static void clear_instance_trace(struct tracefs_instance *instance) > +{ > + FILE *fp; > + char *path; > + > + /* reset the trace */ > + path = tracefs_instance_get_file(instance, "trace"); > + fp = fopen(path, "w"); > + if (!fp) > + die("writing to '%s'", path); > + tracefs_put_tracing_file(path); > + fwrite("0", 1, 1, fp); > + fclose(fp); > +} > + > +static void clear_trace(struct instances_list *instances) > +{ > + if (instances) { > + while (instances) { > + clear_instance_trace(instances->instance); > + instances = instances->next; > + } > + } else > + clear_instance_trace(NULL); > +} > + > +void trace_clear(int argc, char **argv) > +{ > + struct instances_list *instances = NULL; > + bool all = false; > + int c; > + > + for (;;) { > + int option_index = 0; > + static struct option long_options[] = { > + {"all", no_argument, NULL, 'a'}, > + {"help", no_argument, NULL, '?'}, > + {NULL, 0, NULL, 0} > + }; > + > + c = getopt_long (argc-1, argv+1, "+haB:", > + long_options, &option_index); > + if (c == -1) > + break; > + switch (c) { > + case 'B': > + if (add_new_instance(&instances, optarg)) > + die("Failed to allocate instance %s", optarg); > + break; > + case 'a': > + all = true; All you need to do is set the flag, and don't create the list here. > + if (tracefs_instances_walk(add_instance_walk, &instances)) > + die("Failed to add all instances"); > + break; > + case 'h': > + case '?': > + default: > + usage(argv); > + break; > + } > + } > + > + clear_trace(instances); > + if (all) Then here, you would just do: tracefs_instances_iterate(clear_instance_iter, NULL); Where we have: static int clear_instance_iter(const char *name, void *data) { tracefs_clear(name); } No need for the creating of the list. -- Steve > + clear_trace(NULL); > + clear_list(instances); > + exit(0); > +} > + > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index bd004574..5009eaa1 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -835,21 +835,6 @@ static void clear_trace_instances(void) > __clear_trace(instance); > } > > -static void clear_trace(void) > -{ > - FILE *fp; > - char *path; > - > - /* reset the trace */ > - path = tracefs_get_tracing_file("trace"); > - fp = fopen(path, "w"); > - if (!fp) > - die("writing to '%s'", path); > - tracefs_put_tracing_file(path); > - fwrite("0", 1, 1, fp); > - fclose(fp); > -} > - > static void reset_max_latency(struct buffer_instance *instance) > { > tracefs_instance_file_write(instance->tracefs, > @@ -6704,15 +6689,6 @@ void trace_profile(int argc, char **argv) > exit(0); > } > > -void trace_clear(int argc, char **argv) > -{ > - if (argc > 2) > - usage(argv); > - else > - clear_trace(); > - exit(0); > -} > - > void trace_record(int argc, char **argv) > { > struct common_record_context ctx; > diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c > index 3f0b2d07..79610bd3 100644 > --- a/tracecmd/trace-usage.c > +++ b/tracecmd/trace-usage.c > @@ -180,7 +180,9 @@ static struct usage_help usage_help[] = { > { > "clear", > "clear the trace buffers", > - " %s clear\n" > + " %s clear [-B buf][-a]\n" > + " -B clear the given buffer (may specify multiple -B)\n" > + " -a clear all existing buffers, including the top level one\n" > }, > { > "report", ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] trace-cmd: Enhance "trace-cmd clear" to be instance aware 2020-07-17 14:06 ` Steven Rostedt @ 2020-07-17 14:29 ` Tzvetomir Stoyanov 0 siblings, 0 replies; 7+ messages in thread From: Tzvetomir Stoyanov @ 2020-07-17 14:29 UTC (permalink / raw) To: Steven Rostedt; +Cc: Linux Trace Devel On Fri, Jul 17, 2020 at 5:06 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 17 Jul 2020 11:03:26 +0300 > "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote: > > --- /dev/null > > +++ b/tracecmd/trace-clear.c > > @@ -0,0 +1,124 @@ > > +// SPDX-License-Identifier: LGPL-2.1 > > +/* > > + * Copyright (C) 2008, 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> > > + * > > + * Updates: > > + * Copyright (C) 2020, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com> > > + * > > + */ > > +#include <stdlib.h> > > +#include <unistd.h> > > +#include <getopt.h> > > + > > +#include "tracefs.h" > > +#include "trace-local.h" > > + > > +struct instances_list { > > + struct instances_list *next; > > + struct tracefs_instance *instance; > > +}; > > + > > +static int add_new_instance(struct instances_list **list, char *name) > > +{ > > + struct instances_list *new; > > + > > + new = calloc(1, sizeof(*new)); > > + if (!new) > > + return -1; > > + new->instance = tracefs_instance_alloc(name); > > + if (!new->instance) { > > + free(new); > > + return -1; > > + } > > + > > + new->next = *list; > > + *list = new; > > + return 0; > > +} > > + > > +static int add_instance_walk(const char *name, void *data) > > +{ > > + return add_new_instance((struct instances_list **)data, (char *)name); > > +} > > + > > +static void clear_list(struct instances_list *list) > > +{ > > + struct instances_list *del; > > + > > + while (list) { > > + del = list; > > + list = list->next; > > + tracefs_instance_free(del->instance); > > + free(del); > > + } > > +} > > BTW, why create this list? (see below). > > > + > > +static void clear_instance_trace(struct tracefs_instance *instance) > > +{ > > + FILE *fp; > > + char *path; > > + > > + /* reset the trace */ > > + path = tracefs_instance_get_file(instance, "trace"); > > + fp = fopen(path, "w"); > > + if (!fp) > > + die("writing to '%s'", path); > > + tracefs_put_tracing_file(path); > > + fwrite("0", 1, 1, fp); > > + fclose(fp); > > +} > > + > > +static void clear_trace(struct instances_list *instances) > > +{ > > + if (instances) { > > + while (instances) { > > + clear_instance_trace(instances->instance); > > + instances = instances->next; > > + } > > + } else > > + clear_instance_trace(NULL); > > +} > > + > > +void trace_clear(int argc, char **argv) > > +{ > > + struct instances_list *instances = NULL; > > + bool all = false; > > + int c; > > + > > + for (;;) { > > + int option_index = 0; > > + static struct option long_options[] = { > > + {"all", no_argument, NULL, 'a'}, > > + {"help", no_argument, NULL, '?'}, > > + {NULL, 0, NULL, 0} > > + }; > > + > > + c = getopt_long (argc-1, argv+1, "+haB:", > > + long_options, &option_index); > > + if (c == -1) > > + break; > > + switch (c) { > > + case 'B': > > + if (add_new_instance(&instances, optarg)) > > + die("Failed to allocate instance %s", optarg); > > + break; > > + case 'a': > > + all = true; > > All you need to do is set the flag, and don't create the list here. > > > + if (tracefs_instances_walk(add_instance_walk, &instances)) > > + die("Failed to add all instances"); > > + break; > > + case 'h': > > + case '?': > > + default: > > + usage(argv); > > + break; > > + } > > + } > > + > > + clear_trace(instances); > > + if (all) > > Then here, you would just do: > > tracefs_instances_iterate(clear_instance_iter, NULL); > > Where we have: > > static int clear_instance_iter(const char *name, void *data) > { > tracefs_clear(name); > } > > No need for the creating of the list. There are two reasons for creating the list: - For consistency with the other flow, the"-B" option. - The tracefs APIs require "struct tracefs_instance" as input parameter to identify the instance, do not work with instance name The list is not needed for "--all", but still needs to create "struct tracefs_instance" and to pass it to tracefs_instance_get_file() API > > -- Steve > > > > + clear_trace(NULL); > > + clear_list(instances); > > + exit(0); > > +} > > + > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > > index bd004574..5009eaa1 100644 > > --- a/tracecmd/trace-record.c > > +++ b/tracecmd/trace-record.c > > @@ -835,21 +835,6 @@ static void clear_trace_instances(void) > > __clear_trace(instance); > > } > > > > -static void clear_trace(void) > > -{ > > - FILE *fp; > > - char *path; > > - > > - /* reset the trace */ > > - path = tracefs_get_tracing_file("trace"); > > - fp = fopen(path, "w"); > > - if (!fp) > > - die("writing to '%s'", path); > > - tracefs_put_tracing_file(path); > > - fwrite("0", 1, 1, fp); > > - fclose(fp); > > -} > > - > > static void reset_max_latency(struct buffer_instance *instance) > > { > > tracefs_instance_file_write(instance->tracefs, > > @@ -6704,15 +6689,6 @@ void trace_profile(int argc, char **argv) > > exit(0); > > } > > > > -void trace_clear(int argc, char **argv) > > -{ > > - if (argc > 2) > > - usage(argv); > > - else > > - clear_trace(); > > - exit(0); > > -} > > - > > void trace_record(int argc, char **argv) > > { > > struct common_record_context ctx; > > diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c > > index 3f0b2d07..79610bd3 100644 > > --- a/tracecmd/trace-usage.c > > +++ b/tracecmd/trace-usage.c > > @@ -180,7 +180,9 @@ static struct usage_help usage_help[] = { > > { > > "clear", > > "clear the trace buffers", > > - " %s clear\n" > > + " %s clear [-B buf][-a]\n" > > + " -B clear the given buffer (may specify multiple -B)\n" > > + " -a clear all existing buffers, including the top level one\n" > > }, > > { > > "report", > -- Tzvetomir (Ceco) Stoyanov VMware Open Source Technology Center ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-17 14:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-17 8:03 [PATCH 0/2] Enhancements to trace-cmd clear Tzvetomir Stoyanov (VMware) 2020-07-17 8:03 ` [PATCH 1/2] trace-cmd: Add new tracefs API tracefs_instances_walk() Tzvetomir Stoyanov (VMware) 2020-07-17 13:25 ` Steven Rostedt 2020-07-17 8:03 ` [PATCH 2/2] trace-cmd: Enhance "trace-cmd clear" to be instance aware Tzvetomir Stoyanov (VMware) 2020-07-17 13:31 ` Steven Rostedt 2020-07-17 14:06 ` Steven Rostedt 2020-07-17 14:29 ` Tzvetomir Stoyanov
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).