* [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter()
@ 2021-03-30 0:51 Steven Rostedt
2021-03-30 0:51 ` [PATCH 01/13 v2] libtracefs: An API to set the filtering of functions Steven Rostedt
` (13 more replies)
0 siblings, 14 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
Changes since v1:
- tracefs_function_filter() man page was added
- Now thread safe with pthread_mutex_lock()
- Use of flags instead of reset parameter
- Added TRACEFS_FL_CONTINUE flag
- Fixed glob or regex test
Sameeruddin shaik (2):
libtracefs: An API to set the filtering of functions
libtracefs: Document function_filter API
Steven Rostedt (VMware) (11):
libtracefs: Fix notations of tracefs_function_filter() and module parameter
libtracefs: Move opening of file out of controlled_write()
libtracefs: Add add_errors() helper function for control_write()
libtracefs: Add checking of available_filter_functions to tracefs_function_filter()
libtracefs: Add write_filter() helper function
libtracefs: Allow for setting filters with regex expressions
libtracefs: Add indexing to set functions in tracefs_function_filter()
libtracefs: Pass in reset via flags to tracefs_function_filter()
libtracefs: Add pthread_mutex_lock() around tracefs_function_filter()
libtracefs: Move struct tracefs_instance to tracefs-local.h
libtracefs: Add CONTINUE to tracefs_function_filter()
----
Documentation/libtracefs-function-filter.txt | 161 ++++++++
include/tracefs-local.h | 7 +
include/tracefs.h | 11 +
src/tracefs-instance.c | 7 +-
src/tracefs-tools.c | 555 ++++++++++++++++++++++++++-
5 files changed, 735 insertions(+), 6 deletions(-)
create mode 100644 Documentation/libtracefs-function-filter.txt
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 01/13 v2] libtracefs: An API to set the filtering of functions
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 02/13 v2] libtracefs: Document function_filter API Steven Rostedt
` (12 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik, Tzvetomir Stoyanov (VMware)
From: Sameeruddin shaik <sameeruddin.shaik8@gmail.com>
This new API will write the function filters into the
set_ftrace_filter file.
tracefs_function_filter()
https://bugzilla.kernel.org/show_bug.cgi?id=210643
Link: https://lore.kernel.org/linux-trace-devel/1616171938-18914-1-git-send-email-sameeruddin.shaik8@gmail.com
Link: https://lore.kernel.org/linux-trace-devel/20210323013224.729594322@goodmis.org
Acked-by: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
include/tracefs.h | 3 +-
src/tracefs-tools.c | 111 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 113 insertions(+), 1 deletion(-)
diff --git a/include/tracefs.h b/include/tracefs.h
index 05bd0ef4d404..9477fdb14c5a 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -145,5 +145,6 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o
int tracefs_option_enable(struct tracefs_instance *instance, enum tracefs_option_id id);
int tracefs_option_diasble(struct tracefs_instance *instance, enum tracefs_option_id id);
const char *tracefs_option_name(enum tracefs_option_id id);
-
+int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
+ const char *module, bool reset, const char ***errs);
#endif /* _TRACE_FS_H */
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index e2dfc7b8acff..4f8bcdc299df 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -18,6 +18,7 @@
#include "tracefs-local.h"
#define TRACE_CTRL "tracing_on"
+#define TRACE_FILTER "set_ftrace_filter"
static const char * const options_map[] = {
"unknown",
@@ -387,3 +388,113 @@ void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_opt
if (options && id > TRACEFS_OPTION_INVALID)
options->mask &= ~(1ULL << (id - 1));
}
+
+static int controlled_write(const char *filter_path, const char **filters,
+ const char *module, bool reset, const char ***errs)
+{
+ int flags = reset ? O_TRUNC : O_APPEND;
+ const char **temp = NULL;
+ const char **e = NULL;
+ char *each_str = NULL;
+ int write_size = 0;
+ int size = 0;
+ int fd = -1;
+ int ret = 0;
+ int j = 0;
+ int i;
+
+ fd = open(filter_path, O_WRONLY | flags);
+ if (fd < 0)
+ return 1;
+
+ for (i = 0; filters[i]; i++) {
+ if (module)
+ write_size = asprintf(&each_str, "%s:mod:%s ", filters[i], module);
+ else
+ write_size = asprintf(&each_str, "%s ", filters[i]);
+ if (write_size < 0) {
+ ret = 1;
+ goto error;
+ }
+ size = write(fd, each_str, write_size);
+ /* compare written bytes*/
+ if (size < write_size) {
+ if (errs) {
+ temp = realloc(e, (j + 1) * (sizeof(char *)));
+ if (!temp) {
+ free(e);
+ ret = 1;
+ goto error;
+ } else
+ e = temp;
+
+ e[j++] = filters[i];
+ ret -= 1;
+ }
+ }
+ free(each_str);
+ each_str = NULL;
+ }
+ if (errs) {
+ temp = realloc(e, (j + 1) * (sizeof(char *)));
+ if (!temp) {
+ free(e);
+ ret = 1;
+ goto error;
+ } else
+ e = temp;
+ e[j] = NULL;
+ *errs = e;
+ }
+ error:
+ if (each_str)
+ free(each_str);
+ close(fd);
+ return ret;
+}
+
+/**
+ * tracefs_function_filter - write to set_ftrace_filter file to trace
+ * particular functions
+ * @instance: ftrace instance, can be NULL for top tracing instance
+ * @filters: An array of function names ending with a NULL pointer
+ * @module: Module to be traced
+ * @reset: set to true to reset the file before applying the filter
+ * @errs: A pointer to array of constant strings that will be allocated
+ * on negative return of this function, pointing to the filters that
+ * failed.May be NULL, in which case this field will be ignored.
+ *
+ * The @filters is an array of strings, where each string will be used
+ * to set a function or functions to be traced.
+ *
+ * If @reset is true, then all functions in the filter are cleared
+ * before adding functions from @filters. Otherwise, the functions set
+ * by @filters will be appended to the filter file
+ *
+ * returns -x on filter errors (where x is number of failed filter
+ * srtings) and if @errs is not NULL will be an allocated string array
+ * pointing to the strings in @filters that failed and must be freed
+ * with free().
+ *
+ * returns 1 on general errors not realted to setting the filter.
+ * @errs is not set even if supplied.
+ *
+ * return 0 on success and @errs is not set.
+ */
+int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
+ const char *module, bool reset, const char ***errs)
+{
+ char *ftrace_filter_path;
+ int ret = 0;
+
+ if (!filters)
+ return 1;
+
+ ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER);
+ if (!ftrace_filter_path)
+ return 1;
+
+ ret = controlled_write(ftrace_filter_path, filters, module, reset, errs);
+ tracefs_put_tracing_file(ftrace_filter_path);
+ return ret;
+}
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 02/13 v2] libtracefs: Document function_filter API
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
2021-03-30 0:51 ` [PATCH 01/13 v2] libtracefs: An API to set the filtering of functions Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 03/13 v2] libtracefs: Fix notations of tracefs_function_filter() and module parameter Steven Rostedt
` (11 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: Sameeruddin shaik <sameeruddin.shaik8@gmail.com>
Added documentation for the below API:
tracefs_function_filter()
Link: https://lore.kernel.org/linux-trace-devel/1616719420-6747-1-git-send-email-sameeruddin.shaik8@gmail.com
Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com>
[ Cleaned up some white space issues - S.R ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Documentation/libtracefs-function-filter.txt | 120 +++++++++++++++++++
1 file changed, 120 insertions(+)
create mode 100644 Documentation/libtracefs-function-filter.txt
diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
new file mode 100644
index 000000000000..9351788dd1a1
--- /dev/null
+++ b/Documentation/libtracefs-function-filter.txt
@@ -0,0 +1,120 @@
+libtracefs(3)
+=============
+
+NAME
+----
+tracefs_function_filter - Function to limit kernel functions that are traced
+
+SYNOPSIS
+--------
+[verse]
+--
+*#include <tracefs.h>*
+
+int tracefs_function_filter(struct tracefs_instance *instance, const char **filters, const char *module, bool reset, const char ***errs);
+--
+
+DESCRIPTION
+-----------
+This function can be used to limit the Linux kernel functions that were
+traced by the function and function-graph tracers
+
+It will take
+_instance_ , that can be NULL for the top level tracing.
+_filters_, which is an array of the strings that represent a list of filters that should
+be applied to define what functions are to be traced and The array must end
+with a NULL pointer.
+_module_ , name of the module to be traced.
+_reset_ if set will clear the current set of filters and then apply the
+filter list, otherwise the list of filters are added to the current set of
+filters,
+_errs_, is a pointer to an array of strings, which will be allocated if
+any of filters fail to match any available function, If _errs_ is NULL, it will
+be ignored.
+
+returns 0 on success, 1 or -x (where x is an integer) on error.
+
+RETURN VALUE
+------------
+return 0 on success, if there is error, it will return 1 for general errors or
+negative number -x(x denotes number of failed filters), if there are any failed filters.
+
+In case of negative return value, errs have to be checked and must be freed
+using the free()
+
+EXAMPLE
+-------
+[source,c]
+--
+#include <tracefs.h>
+
+#define INST "dummy"
+
+const char *filters[] = { "run_init_process", "try_to_run_init_process", "dummy1", NULL };
+
+int main(int argc, char *argv[])
+{
+ struct tracefs_instance *inst = tracefs_instance_create(INST);
+ const char **errs = NULL;
+ bool reset = 1;
+ int ret;
+ int i = 0;
+
+ if (!inst) {
+ /* Error creating new trace instance*/
+ }
+
+ ret = tracefs_function_filter(inst, filters, NULL, reset, &errs);
+
+ if (ret < 0 && errs) {
+ while (errs[i])
+ printf("%s\n", errs[i++]);
+ }
+
+ tracefs_instance_free(inst);
+ tracefs_instance_destroy(inst);
+ free(errs);
+ return 0;
+}
+--
+
+FILES
+-----
+[verse]
+--
+*tracefs.h*
+ Header file to include in order to have access to the library APIs.
+*-ltracefs*
+ Linker switch to add when building a program that uses the library.
+--
+
+SEE ALSO
+--------
+_libtracefs(3)_,
+_libtraceevent(3)_,
+_trace-cmd(1)_
+
+AUTHOR
+------
+[verse]
+--
+*Steven Rostedt* <rostedt@goodmis.org>
+*Tzvetomir Stoyanov* <tz.stoyanov@gmail.com>
+*sameeruddin shaik* <sameeruddin.shaik8@gmail.com>
+--
+REPORTING BUGS
+--------------
+Report bugs to <linux-trace-devel@vger.kernel.org>
+
+LICENSE
+-------
+libtracefs is Free Software licensed under the GNU LGPL 2.1
+
+RESOURCES
+---------
+https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
+
+COPYING
+-------
+Copyright \(C) 2020 VMware, Inc. Free use of this software is granted under
+the terms of the GNU Public License (GPL).
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 03/13 v2] libtracefs: Fix notations of tracefs_function_filter() and module parameter
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
2021-03-30 0:51 ` [PATCH 01/13 v2] libtracefs: An API to set the filtering of functions Steven Rostedt
2021-03-30 0:51 ` [PATCH 02/13 v2] libtracefs: Document function_filter API Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 04/13 v2] libtracefs: Move opening of file out of controlled_write() Steven Rostedt
` (10 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Highlight the function name and encapsulate each '*' with a "pass:[*]" to
not have the creation of the man pages interpret them as bold annotations.
Also remove the tracefs_instance_free(), as tracefs_instance_destroy() is
called and that frees the instance.
The description in the man page for tracefs_function_filter() left out
that NULL is a suitable value for the modules parameter, meaning that all
functions will be examined for a match of one of the filters.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Documentation/libtracefs-function-filter.txt | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
index 9351788dd1a1..c0c89f372c21 100644
--- a/Documentation/libtracefs-function-filter.txt
+++ b/Documentation/libtracefs-function-filter.txt
@@ -11,7 +11,7 @@ SYNOPSIS
--
*#include <tracefs.h>*
-int tracefs_function_filter(struct tracefs_instance *instance, const char **filters, const char *module, bool reset, const char ***errs);
+int *tracefs_function_filter*(struct tracefs_instance pass:[*]_instance_, const char pass:[*]pass:[*]_filters_, const char pass:[*]_module_, bool _reset_, const char pass:[*]pass:[*]pass:[*]_errs_);
--
DESCRIPTION
@@ -24,7 +24,7 @@ _instance_ , that can be NULL for the top level tracing.
_filters_, which is an array of the strings that represent a list of filters that should
be applied to define what functions are to be traced and The array must end
with a NULL pointer.
-_module_ , name of the module to be traced.
+_module_ , name of the module to be traced (or NULL for all functions),
_reset_ if set will clear the current set of filters and then apply the
filter list, otherwise the list of filters are added to the current set of
filters,
@@ -61,7 +61,7 @@ int main(int argc, char *argv[])
int i = 0;
if (!inst) {
- /* Error creating new trace instance*/
+ /* Error creating new trace instance */
}
ret = tracefs_function_filter(inst, filters, NULL, reset, &errs);
@@ -71,7 +71,6 @@ int main(int argc, char *argv[])
printf("%s\n", errs[i++]);
}
- tracefs_instance_free(inst);
tracefs_instance_destroy(inst);
free(errs);
return 0;
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 04/13 v2] libtracefs: Move opening of file out of controlled_write()
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (2 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 03/13 v2] libtracefs: Fix notations of tracefs_function_filter() and module parameter Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 05/13 v2] libtracefs: Add add_errors() helper function for control_write() Steven Rostedt
` (9 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
In order to try multiple methods of writing to the filter file, the opening
of the filter file needs to be in the tracefs_function_filter() call, and
allow that to be passed to multiple ways of writing to the filter.
Link: https://lore.kernel.org/linux-trace-devel/20210323013224.877520803@goodmis.org
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
src/tracefs-tools.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 4f8bcdc299df..02e43168a810 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -389,24 +389,18 @@ void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_opt
options->mask &= ~(1ULL << (id - 1));
}
-static int controlled_write(const char *filter_path, const char **filters,
- const char *module, bool reset, const char ***errs)
+static int controlled_write(int fd, const char **filters,
+ const char *module, const char ***errs)
{
- int flags = reset ? O_TRUNC : O_APPEND;
const char **temp = NULL;
const char **e = NULL;
char *each_str = NULL;
int write_size = 0;
int size = 0;
- int fd = -1;
int ret = 0;
int j = 0;
int i;
- fd = open(filter_path, O_WRONLY | flags);
- if (fd < 0)
- return 1;
-
for (i = 0; filters[i]; i++) {
if (module)
write_size = asprintf(&each_str, "%s:mod:%s ", filters[i], module);
@@ -449,7 +443,6 @@ static int controlled_write(const char *filter_path, const char **filters,
error:
if (each_str)
free(each_str);
- close(fd);
return ret;
}
@@ -486,6 +479,8 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
{
char *ftrace_filter_path;
int ret = 0;
+ int flags;
+ int fd;
if (!filters)
return 1;
@@ -494,7 +489,16 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
if (!ftrace_filter_path)
return 1;
- ret = controlled_write(ftrace_filter_path, filters, module, reset, errs);
+ flags = reset ? O_TRUNC : O_APPEND;
+
+ fd = open(ftrace_filter_path, O_WRONLY | flags);
tracefs_put_tracing_file(ftrace_filter_path);
+ if (fd < 0)
+ return 1;
+
+ ret = controlled_write(fd, filters, module, errs);
+
+ close(fd);
+
return ret;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 05/13 v2] libtracefs: Add add_errors() helper function for control_write()
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (3 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 04/13 v2] libtracefs: Move opening of file out of controlled_write() Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 06/13 v2] libtracefs: Add checking of available_filter_functions to tracefs_function_filter() Steven Rostedt
` (8 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Move the code that manages errors out of control_write() as this will be
used by other methods in setting the filters.
Link: https://lore.kernel.org/linux-trace-devel/20210323013225.012918584@goodmis.org
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
src/tracefs-tools.c | 61 ++++++++++++++++++++++++---------------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 02e43168a810..1348542a3419 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -389,16 +389,40 @@ void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_opt
options->mask &= ~(1ULL << (id - 1));
}
+static void add_errors(const char ***errs, const char *filter, int ret)
+{
+ const char **e;
+
+ if (!errs)
+ return;
+
+ /* Negative is passed in */
+ ret = -ret;
+ e = *errs;
+
+ /* If this previously failed to allocate stop processing */
+ if (!e && ret)
+ return;
+
+ /* Add 2, one for the new entry, and one for NULL */
+ e = realloc(e, sizeof(*e) * (ret + 2));
+ if (!e) {
+ free(*errs);
+ *errs = NULL;
+ return;
+ }
+ e[ret] = filter;
+ e[ret + 1] = NULL;
+ *errs = e;
+}
+
static int controlled_write(int fd, const char **filters,
const char *module, const char ***errs)
{
- const char **temp = NULL;
- const char **e = NULL;
char *each_str = NULL;
int write_size = 0;
int size = 0;
int ret = 0;
- int j = 0;
int i;
for (i = 0; filters[i]; i++) {
@@ -412,34 +436,11 @@ static int controlled_write(int fd, const char **filters,
}
size = write(fd, each_str, write_size);
/* compare written bytes*/
- if (size < write_size) {
- if (errs) {
- temp = realloc(e, (j + 1) * (sizeof(char *)));
- if (!temp) {
- free(e);
- ret = 1;
- goto error;
- } else
- e = temp;
-
- e[j++] = filters[i];
- ret -= 1;
- }
- }
+ if (size < write_size)
+ add_errors(errs, filters[i], ret--);
free(each_str);
each_str = NULL;
}
- if (errs) {
- temp = realloc(e, (j + 1) * (sizeof(char *)));
- if (!temp) {
- free(e);
- ret = 1;
- goto error;
- } else
- e = temp;
- e[j] = NULL;
- *errs = e;
- }
error:
if (each_str)
free(each_str);
@@ -496,6 +497,10 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
if (fd < 0)
return 1;
+ /* Make sure errs is NULL to start with, realloc() depends on it. */
+ if (errs)
+ *errs = NULL;
+
ret = controlled_write(fd, filters, module, errs);
close(fd);
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 06/13 v2] libtracefs: Add checking of available_filter_functions to tracefs_function_filter()
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (4 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 05/13 v2] libtracefs: Add add_errors() helper function for control_write() Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 07/13 v2] libtracefs: Add write_filter() helper function Steven Rostedt
` (7 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Instead of modifying the set_ftrace_filter file with a filter that
possibly does not match anything, check the available_filter_functions
file first to make sure that each filter will match something. Then if
"reset" is enabled, it wont clear the filter file and not set any
filtering, which could cause issues if the application is not expecting
all functions to be enabled.
Link: https://lore.kernel.org/linux-trace-devel/20210323013225.156539389@goodmis.org
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
src/tracefs-tools.c | 183 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 178 insertions(+), 5 deletions(-)
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 1348542a3419..db12f759be1b 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -10,6 +10,7 @@
#include <unistd.h>
#include <fcntl.h>
#include <sys/types.h>
+#include <regex.h>
#include <dirent.h>
#include <limits.h>
#include <errno.h>
@@ -416,6 +417,112 @@ static void add_errors(const char ***errs, const char *filter, int ret)
*errs = e;
}
+struct func_filter {
+ const char *filter;
+ regex_t re;
+ bool set;
+};
+
+/*
+ * Convert a glob into a regular expression.
+ */
+static char *make_regex(const char *glob)
+{
+ char *str;
+ int cnt = 0;
+ int i, j;
+
+ for (i = 0; glob[i]; i++) {
+ if (glob[i] == '*'|| glob[i] == '.')
+ cnt++;
+ }
+
+ /* '^' + ('*'->'.*' or '.' -> '\.') + '$' + '\0' */
+ str = malloc(i + cnt + 3);
+ if (!str)
+ return NULL;
+
+ str[0] = '^';
+ for (i = 0, j = 1; glob[i]; i++, j++) {
+ if (glob[i] == '*')
+ str[j++] = '.';
+ /* Dots can be part of a function name */
+ if (glob[i] == '.')
+ str[j++] = '\\';
+ str[j] = glob[i];
+ }
+ str[j++] = '$';
+ str[j] = '\0';
+ return str;
+}
+
+static bool match(const char *str, struct func_filter *func_filter)
+{
+ return regexec(&func_filter->re, str, 0, NULL, 0) == 0;
+}
+
+static int check_available_filters(struct func_filter *func_filters,
+ const char *module, const char ***errs)
+{
+ char *line = NULL;
+ size_t size = 0;
+ char *path;
+ FILE *fp;
+ int ret = 1;
+ int mlen;
+ int i;
+
+ path = tracefs_get_tracing_file("available_filter_functions");
+ if (!path)
+ return 1;
+
+ fp = fopen(path, "r");
+ tracefs_put_tracing_file(path);
+
+ if (!fp)
+ return 1;
+
+ if (module)
+ mlen = strlen(module);
+
+ while (getline(&line, &size, fp) >= 0) {
+ char *saveptr = NULL;
+ char *tok, *mtok;
+ int len = strlen(line);
+
+ if (line[len - 1] == '\n')
+ line[len - 1] = '\0';
+ tok = strtok_r(line, " ", &saveptr);
+ if (!tok)
+ goto next;
+ if (module) {
+ mtok = strtok_r(NULL, " ", &saveptr);
+ if (!mtok)
+ goto next;
+ if ((strncmp(mtok + 1, module, mlen) != 0) ||
+ (mtok[mlen + 1] != ']'))
+ goto next;
+ }
+ for (i = 0; func_filters[i].filter; i++) {
+ if (match(tok, &func_filters[i]))
+ func_filters[i].set = true;
+ }
+ next:
+ free(line);
+ line = NULL;
+ len = 0;
+ }
+ fclose(fp);
+
+ ret = 0;
+ for (i = 0; func_filters[i].filter; i++) {
+ if (!func_filters[i].set)
+ add_errors(errs, func_filters[i].filter, ret--);
+ }
+
+ return ret;
+}
+
static int controlled_write(int fd, const char **filters,
const char *module, const char ***errs)
{
@@ -447,6 +554,62 @@ static int controlled_write(int fd, const char **filters,
return ret;
}
+static int init_func_filter(struct func_filter *func_filter, const char *filter)
+{
+ char *str;
+ int ret;
+
+ str = make_regex(filter);
+ if (!str)
+ return -1;
+
+ ret = regcomp(&func_filter->re, str, REG_ICASE|REG_NOSUB);
+ free(str);
+
+ if (ret < 0)
+ return -1;
+
+ func_filter->filter = filter;
+ return 0;
+}
+
+static void free_func_filters(struct func_filter *func_filters)
+{
+ int i;
+
+ if (!func_filters)
+ return;
+
+ for (i = 0; func_filters[i].filter; i++) {
+ regfree(&func_filters[i].re);
+ }
+}
+
+static struct func_filter *make_func_filters(const char **filters)
+{
+ struct func_filter *func_filters = NULL;
+ int i;
+
+ for (i = 0; filters[i]; i++)
+ ;
+
+ if (!i)
+ return NULL;
+
+ func_filters = calloc(i + 1, sizeof(*func_filters));
+ if (!func_filters)
+ return NULL;
+
+ for (i = 0; filters[i]; i++) {
+ if (init_func_filter(&func_filters[i], filters[i]) < 0)
+ goto out_err;
+ }
+ return func_filters;
+ out_err:
+ free_func_filters(func_filters);
+ return NULL;
+}
+
/**
* tracefs_function_filter - write to set_ftrace_filter file to trace
* particular functions
@@ -478,14 +641,28 @@ static int controlled_write(int fd, const char **filters,
int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
const char *module, bool reset, const char ***errs)
{
+ struct func_filter *func_filters;
char *ftrace_filter_path;
- int ret = 0;
int flags;
+ int ret;
int fd;
if (!filters)
return 1;
+ func_filters = make_func_filters(filters);
+ if (!func_filters)
+ return 1;
+
+ /* Make sure errs is NULL to start with, realloc() depends on it. */
+ if (errs)
+ *errs = NULL;
+
+ ret = check_available_filters(func_filters, module, errs);
+ free_func_filters(func_filters);
+ if (ret)
+ return ret;
+
ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER);
if (!ftrace_filter_path)
return 1;
@@ -497,10 +674,6 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
if (fd < 0)
return 1;
- /* Make sure errs is NULL to start with, realloc() depends on it. */
- if (errs)
- *errs = NULL;
-
ret = controlled_write(fd, filters, module, errs);
close(fd);
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 07/13 v2] libtracefs: Add write_filter() helper function
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (5 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 06/13 v2] libtracefs: Add checking of available_filter_functions to tracefs_function_filter() Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 08/13 v2] libtracefs: Allow for setting filters with regex expressions Steven Rostedt
` (6 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Wrap the writing of the filter into the filter file in a helper function
such that it can be used in more places.
Link: https://lore.kernel.org/linux-trace-devel/20210323013225.303716095@goodmis.org
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
src/tracefs-tools.c | 58 ++++++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 19 deletions(-)
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index db12f759be1b..470502b07f7d 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -461,6 +461,33 @@ static bool match(const char *str, struct func_filter *func_filter)
return regexec(&func_filter->re, str, 0, NULL, 0) == 0;
}
+/*
+ * Return 0 on success, -1 error writing, 1 on other errors.
+ */
+static int write_filter(int fd, const char *filter, const char *module)
+{
+ char *each_str = NULL;
+ int write_size;
+ int size;
+
+ if (module)
+ write_size = asprintf(&each_str, "%s:mod:%s ", filter, module);
+ else
+ write_size = asprintf(&each_str, "%s ", filter);
+
+ if (write_size < 0)
+ return 1;
+
+ size = write(fd, each_str, write_size);
+ free(each_str);
+
+ /* compare written bytes*/
+ if (size < write_size)
+ return -1;
+
+ return 0;
+}
+
static int check_available_filters(struct func_filter *func_filters,
const char *module, const char ***errs)
{
@@ -526,31 +553,24 @@ static int check_available_filters(struct func_filter *func_filters,
static int controlled_write(int fd, const char **filters,
const char *module, const char ***errs)
{
- char *each_str = NULL;
- int write_size = 0;
- int size = 0;
int ret = 0;
int i;
for (i = 0; filters[i]; i++) {
- if (module)
- write_size = asprintf(&each_str, "%s:mod:%s ", filters[i], module);
- else
- write_size = asprintf(&each_str, "%s ", filters[i]);
- if (write_size < 0) {
- ret = 1;
- goto error;
- }
- size = write(fd, each_str, write_size);
- /* compare written bytes*/
- if (size < write_size)
+ int r;
+
+ r = write_filter(fd, filters[i], module);
+ if (r < 0) {
add_errors(errs, filters[i], ret--);
- free(each_str);
- each_str = NULL;
+ } else if (r > 0) {
+ /* Not filter error */
+ if (errs) {
+ free(*errs);
+ *errs = NULL;
+ }
+ return 1;
+ }
}
- error:
- if (each_str)
- free(each_str);
return ret;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 08/13 v2] libtracefs: Allow for setting filters with regex expressions
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (6 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 07/13 v2] libtracefs: Add write_filter() helper function Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-04-01 16:33 ` sameeruddin shaik
2021-03-30 0:51 ` [PATCH 09/13 v2] libtracefs: Add indexing to set functions in tracefs_function_filter() Steven Rostedt
` (5 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
All for full "regex(3)" processing of setting functions in the
set_ftrace_filter file. Check if the filter passed in is just a glob
expression that the kernel can process, or if it is a regex that should look
at the available_filter_functions list instead.
If it is a regex, it will read the available_filter_functions and write in
each function as it finds it.
Link: https://lore.kernel.org/linux-trace-devel/20210323013225.451281989@goodmis.org
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Documentation/libtracefs-function-filter.txt | 10 ++
src/tracefs-tools.c | 139 ++++++++++++++++---
2 files changed, 128 insertions(+), 21 deletions(-)
diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
index c0c89f372c21..88aa3b923d54 100644
--- a/Documentation/libtracefs-function-filter.txt
+++ b/Documentation/libtracefs-function-filter.txt
@@ -32,6 +32,16 @@ _errs_, is a pointer to an array of strings, which will be allocated if
any of filters fail to match any available function, If _errs_ is NULL, it will
be ignored.
+A filter in the array of _filters_ may be either a straight match of a
+function, a glob or regex(3). a glob is where '*' matches zero or more
+characters, '?' will match zero or one character, and '.' only matches a
+period. If the filter is determined to be a regex (where it contains
+anything other than alpha numeric characters, or '.', '*', '?') the filter
+will be processed as a regex(3) following the rules of regex(3), and '.' is
+not a period, but will match any one character. To force a regular
+expression, either prefix the filter with a '^' or append it with a '$' as
+all filters will act as complete matches of functions anyway.
+
returns 0 on success, 1 or -x (where x is an integer) on error.
RETURN VALUE
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 470502b07f7d..d1a448459c6f 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -18,8 +18,9 @@
#include "tracefs.h"
#include "tracefs-local.h"
-#define TRACE_CTRL "tracing_on"
-#define TRACE_FILTER "set_ftrace_filter"
+#define TRACE_CTRL "tracing_on"
+#define TRACE_FILTER "set_ftrace_filter"
+#define TRACE_FILTER_LIST "available_filter_functions"
static const char * const options_map[] = {
"unknown",
@@ -421,8 +422,53 @@ struct func_filter {
const char *filter;
regex_t re;
bool set;
+ bool is_regex;
};
+static bool is_regex(const char *str)
+{
+ int i;
+
+ for (i = 0; str[i]; i++) {
+ switch (str[i]) {
+ case 'a' ... 'z':
+ case 'A'...'Z':
+ case '_':
+ case '0'...'9':
+ case '*':
+ case '.':
+ /* Dots can be part of a function name */
+ case '?':
+ continue;
+ default:
+ return true;
+ }
+ }
+ return false;
+}
+
+static char *update_regex(const char *reg)
+{
+ int len = strlen(reg);
+ char *str;
+
+ if (reg[0] == '^' && reg[len - 1] == '$')
+ return strdup(reg);
+
+ str = malloc(len + 3);
+ if (reg[0] == '^') {
+ strcpy(str, reg);
+ } else {
+ str[0] = '^';
+ strcpy(str + 1, reg);
+ len++; /* add ^ */
+ }
+ if (str[len - 1] != '$')
+ str[len++]= '$';
+ str[len] = '\0';
+ return str;
+}
+
/*
* Convert a glob into a regular expression.
*/
@@ -488,8 +534,13 @@ static int write_filter(int fd, const char *filter, const char *module)
return 0;
}
-static int check_available_filters(struct func_filter *func_filters,
- const char *module, const char ***errs)
+enum match_type {
+ FILTER_CHECK,
+ FILTER_WRITE,
+};
+
+static int match_filters(int fd, struct func_filter *func_filters,
+ const char *module, enum match_type type)
{
char *line = NULL;
size_t size = 0;
@@ -499,7 +550,7 @@ static int check_available_filters(struct func_filter *func_filters,
int mlen;
int i;
- path = tracefs_get_tracing_file("available_filter_functions");
+ path = tracefs_get_tracing_file(TRACE_FILTER_LIST);
if (!path)
return 1;
@@ -530,39 +581,76 @@ static int check_available_filters(struct func_filter *func_filters,
(mtok[mlen + 1] != ']'))
goto next;
}
- for (i = 0; func_filters[i].filter; i++) {
- if (match(tok, &func_filters[i]))
- func_filters[i].set = true;
+ switch (type) {
+ case FILTER_CHECK:
+ /* Check, checks a list of filters */
+ for (i = 0; func_filters[i].filter; i++) {
+ if (match(tok, &func_filters[i]))
+ func_filters[i].set = true;
+ }
+ break;
+ case FILTER_WRITE:
+ /* Writes only have one filter */
+ if (match(tok, func_filters)) {
+ ret = write_filter(fd, tok, module);
+ if (ret)
+ goto out;
+ }
+ break;
}
next:
free(line);
line = NULL;
len = 0;
}
+ out:
+ free(line);
fclose(fp);
+ return ret;
+}
+
+static int check_available_filters(struct func_filter *func_filters,
+ const char *module, const char ***errs)
+{
+ int ret;
+ int i;
+
+ ret = match_filters(-1, func_filters, module, FILTER_CHECK);
+ /* Return here if success or non filter error */
+ if (ret >= 0)
+ return ret;
+
+ /* Failed on filter, set the errors */
ret = 0;
for (i = 0; func_filters[i].filter; i++) {
if (!func_filters[i].set)
add_errors(errs, func_filters[i].filter, ret--);
}
-
return ret;
}
-static int controlled_write(int fd, const char **filters,
+static int set_regex_filter(int fd, struct func_filter *func_filter,
+ const char *module)
+{
+ return match_filters(fd, func_filter, module, FILTER_WRITE);
+}
+
+static int controlled_write(int fd, struct func_filter *func_filters,
const char *module, const char ***errs)
{
int ret = 0;
int i;
- for (i = 0; filters[i]; i++) {
+ for (i = 0; func_filters[i].filter; i++) {
+ const char *filter = func_filters[i].filter;
int r;
- r = write_filter(fd, filters[i], module);
- if (r < 0) {
- add_errors(errs, filters[i], ret--);
- } else if (r > 0) {
+ if (func_filters[i].is_regex)
+ r = set_regex_filter(fd, &func_filters[i], module);
+ else
+ r = write_filter(fd, filter, module);
+ if (r > 0) {
/* Not filter error */
if (errs) {
free(*errs);
@@ -570,6 +658,8 @@ static int controlled_write(int fd, const char **filters,
}
return 1;
}
+ if (r < 0)
+ add_errors(errs, filter, ret--);
}
return ret;
}
@@ -579,7 +669,11 @@ static int init_func_filter(struct func_filter *func_filter, const char *filter)
char *str;
int ret;
- str = make_regex(filter);
+ if (!(func_filter->is_regex = is_regex(filter)))
+ str = make_regex(filter);
+ else
+ str = update_regex(filter);
+
if (!str)
return -1;
@@ -679,24 +773,27 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
*errs = NULL;
ret = check_available_filters(func_filters, module, errs);
- free_func_filters(func_filters);
if (ret)
- return ret;
+ goto out_free;
+ ret = 1;
ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER);
if (!ftrace_filter_path)
- return 1;
+ goto out_free;
flags = reset ? O_TRUNC : O_APPEND;
fd = open(ftrace_filter_path, O_WRONLY | flags);
tracefs_put_tracing_file(ftrace_filter_path);
if (fd < 0)
- return 1;
+ goto out_free;
- ret = controlled_write(fd, filters, module, errs);
+ ret = controlled_write(fd, func_filters, module, errs);
close(fd);
+ out_free:
+ free_func_filters(func_filters);
+
return ret;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 09/13 v2] libtracefs: Add indexing to set functions in tracefs_function_filter()
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (7 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 08/13 v2] libtracefs: Allow for setting filters with regex expressions Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 10/13 v2] libtracefs: Pass in reset via flags to tracefs_function_filter() Steven Rostedt
` (4 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Starting in Linux kernel 5.1, it is possible to write the index of the
available_filter_functions (where the first function is 1), which is a
much faster way to set functions than with the glob expressions. By
finding the matches of the available_filter_functions and using the index
of the function, this can make the function call of
tracefs_function_filter() return much faster.
Link: https://lore.kernel.org/linux-trace-devel/20210323013225.595028176@goodmis.org
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
src/tracefs-tools.c | 123 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 116 insertions(+), 7 deletions(-)
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index d1a448459c6f..8f2130948fe0 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -418,6 +418,12 @@ static void add_errors(const char ***errs, const char *filter, int ret)
*errs = e;
}
+struct func_list {
+ struct func_list *next;
+ unsigned int start;
+ unsigned int end;
+};
+
struct func_filter {
const char *filter;
regex_t re;
@@ -534,18 +540,54 @@ static int write_filter(int fd, const char *filter, const char *module)
return 0;
}
+static int add_func(struct func_list ***next_func_ptr, unsigned int index)
+{
+ struct func_list **next_func = *next_func_ptr;
+ struct func_list *func_list = *next_func;
+
+ if (!func_list) {
+ func_list = calloc(1, sizeof(*func_list));
+ if (!func_list)
+ return -1;
+ func_list->start = index;
+ func_list->end = index;
+ *next_func = func_list;
+ return 0;
+ }
+
+ if (index == func_list->end + 1) {
+ func_list->end = index;
+ return 0;
+ }
+ *next_func_ptr = &func_list->next;
+ return add_func(next_func_ptr, index);
+}
+
+static void free_func_list(struct func_list *func_list)
+{
+ struct func_list *f;
+
+ while (func_list) {
+ f = func_list;
+ func_list = f->next;
+ free(f);
+ }
+}
+
enum match_type {
FILTER_CHECK,
FILTER_WRITE,
};
static int match_filters(int fd, struct func_filter *func_filters,
- const char *module, enum match_type type)
+ const char *module, struct func_list **func_list,
+ enum match_type type)
{
char *line = NULL;
size_t size = 0;
char *path;
FILE *fp;
+ int index = 0;
int ret = 1;
int mlen;
int i;
@@ -567,12 +609,16 @@ static int match_filters(int fd, struct func_filter *func_filters,
char *saveptr = NULL;
char *tok, *mtok;
int len = strlen(line);
+ bool first = true;
if (line[len - 1] == '\n')
line[len - 1] = '\0';
tok = strtok_r(line, " ", &saveptr);
if (!tok)
goto next;
+
+ index++;
+
if (module) {
mtok = strtok_r(NULL, " ", &saveptr);
if (!mtok)
@@ -585,8 +631,23 @@ static int match_filters(int fd, struct func_filter *func_filters,
case FILTER_CHECK:
/* Check, checks a list of filters */
for (i = 0; func_filters[i].filter; i++) {
- if (match(tok, &func_filters[i]))
+ /*
+ * If a match was found, still need to
+ * check if other filters would match
+ * to make sure that all filters have a
+ * match, as some filters may overlap.
+ */
+ if (!first && func_filters[i].set)
+ continue;
+ if (match(tok, &func_filters[i])) {
func_filters[i].set = true;
+ if (first) {
+ first = false;
+ ret = add_func(&func_list, index);
+ if (ret)
+ goto out;
+ }
+ }
}
break;
case FILTER_WRITE:
@@ -611,12 +672,13 @@ static int match_filters(int fd, struct func_filter *func_filters,
}
static int check_available_filters(struct func_filter *func_filters,
- const char *module, const char ***errs)
+ const char *module, const char ***errs,
+ struct func_list **func_list)
{
int ret;
int i;
- ret = match_filters(-1, func_filters, module, FILTER_CHECK);
+ ret = match_filters(-1, func_filters, module, func_list, FILTER_CHECK);
/* Return here if success or non filter error */
if (ret >= 0)
return ret;
@@ -633,7 +695,7 @@ static int check_available_filters(struct func_filter *func_filters,
static int set_regex_filter(int fd, struct func_filter *func_filter,
const char *module)
{
- return match_filters(fd, func_filter, module, FILTER_WRITE);
+ return match_filters(fd, func_filter, module, NULL, FILTER_WRITE);
}
static int controlled_write(int fd, struct func_filter *func_filters,
@@ -724,6 +786,49 @@ static struct func_filter *make_func_filters(const char **filters)
return NULL;
}
+static int write_number(int fd, unsigned int start, unsigned int end)
+{
+ char buf[64];
+ unsigned int i;
+ int n, ret;
+
+ for (i = start; i <= end; i++) {
+ n = snprintf(buf, 64, "%d ", i);
+ ret = write(fd, buf, n);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+/*
+ * This will try to write the first number, if that fails, it
+ * will assume that it is not supported and return 1.
+ * If the first write succeeds, but a following write fails, then
+ * the kernel does support this, but something else went wrong,
+ * in this case, return -1.
+ */
+static int write_func_list(int fd, struct func_list *list)
+{
+ int ret;
+
+ if (!list)
+ return 0;
+
+ ret = write_number(fd, list->start, list->end);
+ if (ret)
+ return 1; // try a different way
+ list = list->next;
+ while (list) {
+ ret = write_number(fd, list->start, list->end);
+ if (ret)
+ return -1;
+ list = list->next;
+ }
+ return 0;
+}
+
/**
* tracefs_function_filter - write to set_ftrace_filter file to trace
* particular functions
@@ -756,6 +861,7 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
const char *module, bool reset, const char ***errs)
{
struct func_filter *func_filters;
+ struct func_list *func_list = NULL;
char *ftrace_filter_path;
int flags;
int ret;
@@ -772,7 +878,7 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
if (errs)
*errs = NULL;
- ret = check_available_filters(func_filters, module, errs);
+ ret = check_available_filters(func_filters, module, errs, &func_list);
if (ret)
goto out_free;
@@ -788,11 +894,14 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
if (fd < 0)
goto out_free;
- ret = controlled_write(fd, func_filters, module, errs);
+ ret = write_func_list(fd, func_list);
+ if (ret > 0)
+ ret = controlled_write(fd, func_filters, module, errs);
close(fd);
out_free:
+ free_func_list(func_list);
free_func_filters(func_filters);
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 10/13 v2] libtracefs: Pass in reset via flags to tracefs_function_filter()
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (8 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 09/13 v2] libtracefs: Add indexing to set functions in tracefs_function_filter() Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 14:29 ` Tzvetomir Stoyanov
2021-03-30 0:51 ` [PATCH 11/13 v2] libtracefs: Add pthread_mutex_lock() around tracefs_function_filter() Steven Rostedt
` (3 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Instead of having a separate boolean to tell tracefs_function_filter() to
reset the file upon opening, make it a flag instead. This way other
booleans can be passed into the function without having to extend the
parameters.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Documentation/libtracefs-function-filter.txt | 26 ++++++++++++++------
include/tracefs.h | 10 +++++++-
src/tracefs-tools.c | 19 ++++++++------
3 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
index 88aa3b923d54..5b55a72727c8 100644
--- a/Documentation/libtracefs-function-filter.txt
+++ b/Documentation/libtracefs-function-filter.txt
@@ -11,7 +11,7 @@ SYNOPSIS
--
*#include <tracefs.h>*
-int *tracefs_function_filter*(struct tracefs_instance pass:[*]_instance_, const char pass:[*]pass:[*]_filters_, const char pass:[*]_module_, bool _reset_, const char pass:[*]pass:[*]pass:[*]_errs_);
+int *tracefs_function_filter*(struct tracefs_instance pass:[*]_instance_, const char pass:[*]pass:[*]_filters_, const char pass:[*]_module_, int _flags_, const char pass:[*]pass:[*]pass:[*]_errs_);
--
DESCRIPTION
@@ -25,18 +25,17 @@ _filters_, which is an array of the strings that represent a list of filters tha
be applied to define what functions are to be traced and The array must end
with a NULL pointer.
_module_ , name of the module to be traced (or NULL for all functions),
-_reset_ if set will clear the current set of filters and then apply the
-filter list, otherwise the list of filters are added to the current set of
-filters,
+_flags_ which holds control knobs on how the filters will be handled (see *FLAGS*)
+section below,
_errs_, is a pointer to an array of strings, which will be allocated if
any of filters fail to match any available function, If _errs_ is NULL, it will
be ignored.
A filter in the array of _filters_ may be either a straight match of a
-function, a glob or regex(3). a glob is where '*' matches zero or more
+function, a glob or regex(3). a glob is where 'pass:[*]' matches zero or more
characters, '?' will match zero or one character, and '.' only matches a
period. If the filter is determined to be a regex (where it contains
-anything other than alpha numeric characters, or '.', '*', '?') the filter
+anything other than alpha numeric characters, or '.', 'pass:[*]', '?') the filter
will be processed as a regex(3) following the rules of regex(3), and '.' is
not a period, but will match any one character. To force a regular
expression, either prefix the filter with a '^' or append it with a '$' as
@@ -44,6 +43,17 @@ all filters will act as complete matches of functions anyway.
returns 0 on success, 1 or -x (where x is an integer) on error.
+FLAGS
+-----
+
+The _flags_ parameter may have the following set, or be zero.
+
+*TRACEFS_FL_RESET* :
+If _flags_ contains *TRACEFS_FL_RESET*, then it will clear the filters that
+are currently set before applying the list of filters from _filters_. Otherwise,
+the list of filters from _filters_ will be added to the current set of filters
+already enabled.
+
RETURN VALUE
------------
return 0 on success, if there is error, it will return 1 for general errors or
@@ -66,7 +76,7 @@ int main(int argc, char *argv[])
{
struct tracefs_instance *inst = tracefs_instance_create(INST);
const char **errs = NULL;
- bool reset = 1;
+ int flags = TRACEFS_FL_RESET;
int ret;
int i = 0;
@@ -74,7 +84,7 @@ int main(int argc, char *argv[])
/* Error creating new trace instance */
}
- ret = tracefs_function_filter(inst, filters, NULL, reset, &errs);
+ ret = tracefs_function_filter(inst, filters, NULL, flags, &errs);
if (ret < 0 && errs) {
while (errs[i])
diff --git a/include/tracefs.h b/include/tracefs.h
index 9477fdb14c5a..5193d46f41f5 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -145,6 +145,14 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o
int tracefs_option_enable(struct tracefs_instance *instance, enum tracefs_option_id id);
int tracefs_option_diasble(struct tracefs_instance *instance, enum tracefs_option_id id);
const char *tracefs_option_name(enum tracefs_option_id id);
+
+/*
+ * RESET - Reset on opening filter file (O_TRUNC)
+ */
+enum {
+ TRACEFS_FL_RESET = (1 << 0),
+};
+
int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
- const char *module, bool reset, const char ***errs);
+ const char *module, unsigned int flags, const char ***errs);
#endif /* _TRACE_FS_H */
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 8f2130948fe0..6d03b4856a63 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -835,7 +835,7 @@ static int write_func_list(int fd, struct func_list *list)
* @instance: ftrace instance, can be NULL for top tracing instance
* @filters: An array of function names ending with a NULL pointer
* @module: Module to be traced
- * @reset: set to true to reset the file before applying the filter
+ * @flags: flags on modifying the filter file
* @errs: A pointer to array of constant strings that will be allocated
* on negative return of this function, pointing to the filters that
* failed.May be NULL, in which case this field will be ignored.
@@ -843,9 +843,11 @@ static int write_func_list(int fd, struct func_list *list)
* The @filters is an array of strings, where each string will be used
* to set a function or functions to be traced.
*
- * If @reset is true, then all functions in the filter are cleared
- * before adding functions from @filters. Otherwise, the functions set
- * by @filters will be appended to the filter file
+ * @flags:
+ * TRACEFS_FL_RESET - will clear the functions in the filter file
+ * before applying the @filters. This flag is ignored
+ * if this function is called again when the previous
+ * call had TRACEFS_FL_CONTINUE set.
*
* returns -x on filter errors (where x is number of failed filter
* srtings) and if @errs is not NULL will be an allocated string array
@@ -858,12 +860,13 @@ static int write_func_list(int fd, struct func_list *list)
* return 0 on success and @errs is not set.
*/
int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
- const char *module, bool reset, const char ***errs)
+ const char *module, unsigned int flags, const char ***errs)
{
struct func_filter *func_filters;
struct func_list *func_list = NULL;
char *ftrace_filter_path;
- int flags;
+ bool reset = flags & TRACEFS_FL_RESET;
+ int open_flags;
int ret;
int fd;
@@ -887,9 +890,9 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
if (!ftrace_filter_path)
goto out_free;
- flags = reset ? O_TRUNC : O_APPEND;
+ open_flags = reset ? O_TRUNC : O_APPEND;
- fd = open(ftrace_filter_path, O_WRONLY | flags);
+ fd = open(ftrace_filter_path, O_WRONLY | open_flags);
tracefs_put_tracing_file(ftrace_filter_path);
if (fd < 0)
goto out_free;
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 11/13 v2] libtracefs: Add pthread_mutex_lock() around tracefs_function_filter()
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (9 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 10/13 v2] libtracefs: Pass in reset via flags to tracefs_function_filter() Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 12/13 v2] libtracefs: Move struct tracefs_instance to tracefs-local.h Steven Rostedt
` (2 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
In order to make tracefs_function_filter() thread safe, add a pthread
mutex around the entire function to only let one thread access it at a
time.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
src/tracefs-tools.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 6d03b4856a63..7e191e207867 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -13,6 +13,7 @@
#include <regex.h>
#include <dirent.h>
#include <limits.h>
+#include <pthread.h>
#include <errno.h>
#include "tracefs.h"
@@ -22,6 +23,8 @@
#define TRACE_FILTER "set_ftrace_filter"
#define TRACE_FILTER_LIST "available_filter_functions"
+static pthread_mutex_t filter_lock = PTHREAD_MUTEX_INITIALIZER;
+
static const char * const options_map[] = {
"unknown",
"annotate",
@@ -867,15 +870,16 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
char *ftrace_filter_path;
bool reset = flags & TRACEFS_FL_RESET;
int open_flags;
- int ret;
+ int ret = 1;
int fd;
+ pthread_mutex_lock(&filter_lock);
if (!filters)
- return 1;
+ goto out;
func_filters = make_func_filters(filters);
if (!func_filters)
- return 1;
+ goto out;
/* Make sure errs is NULL to start with, realloc() depends on it. */
if (errs)
@@ -906,6 +910,8 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
out_free:
free_func_list(func_list);
free_func_filters(func_filters);
+ out:
+ pthread_mutex_unlock(&filter_lock);
return ret;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 12/13 v2] libtracefs: Move struct tracefs_instance to tracefs-local.h
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (10 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 11/13 v2] libtracefs: Add pthread_mutex_lock() around tracefs_function_filter() Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter() Steven Rostedt
2021-03-30 1:03 ` [RFC][PATCH 14/13 v2] libtracefs: Just past one filter in for tracefs_function_filter() Steven Rostedt
13 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
In order to have direct access to the tracefs_instance structure for all
of the libtracefs internal code, move it to the tracefs-local.h file, then
other parts of libtracefs code can use it to store internal data.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
include/tracefs-local.h | 6 ++++++
src/tracefs-instance.c | 5 -----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/tracefs-local.h b/include/tracefs-local.h
index 187870e7c491..9c18218cd916 100644
--- a/include/tracefs-local.h
+++ b/include/tracefs-local.h
@@ -14,6 +14,12 @@
#define BUILD_BUG_ON(cond) \
do { if (!(1/!(cond))) { } } while (0)
+struct tracefs_instance {
+ char *trace_dir;
+ char *name;
+ int flags;
+};
+
/* Can be overridden */
void warning(const char *fmt, ...);
diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c
index 0df313c8e8c6..a02c839f2079 100644
--- a/src/tracefs-instance.c
+++ b/src/tracefs-instance.c
@@ -19,11 +19,6 @@
#include "tracefs-local.h"
#define FLAG_INSTANCE_NEWLY_CREATED (1 << 0)
-struct tracefs_instance {
- char *trace_dir;
- char *name;
- int flags;
-};
/**
* instance_alloc - allocate a new ftrace instance
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter()
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (11 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 12/13 v2] libtracefs: Move struct tracefs_instance to tracefs-local.h Steven Rostedt
@ 2021-03-30 0:51 ` Steven Rostedt
2021-03-30 14:29 ` Tzvetomir Stoyanov
2021-03-30 1:03 ` [RFC][PATCH 14/13 v2] libtracefs: Just past one filter in for tracefs_function_filter() Steven Rostedt
13 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 0:51 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Add the TRACEFS_FL_CONTINUE flag to the tracefs_function_filter() API that
will allow it to return without closing the set_ftrace_filter file. When
the set_ftrace_filter file is closed, all the changes to it take place,
but not before hand. In the case that multiple modules need to be set in
one activation, the tracefs_function_filter() would need to be called
multiple times without closing the file descriptor.
Note, the next call to tracefs_function_filter() after it was called with
the CONTINUE flag set, the RESET flag is ignored, as the RESET flag only
takes effect on opening the file. The next call to
tracefs_function_filter() after it was called with the CONTINUE flag (on
the same instance) does not reopen the file, and thus will not reset the
file.
If the file is opened, calling tracefs_function_filter() with no filters
and the continue flag not set, will simply close the set_ftrace_filter
file.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Documentation/libtracefs-function-filter.txt | 32 +++++++++++++---
include/tracefs-local.h | 1 +
include/tracefs.h | 2 +
src/tracefs-instance.c | 2 +
src/tracefs-tools.c | 39 ++++++++++++++++----
5 files changed, 64 insertions(+), 12 deletions(-)
diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
index 5b55a72727c8..1ac8a06961bf 100644
--- a/Documentation/libtracefs-function-filter.txt
+++ b/Documentation/libtracefs-function-filter.txt
@@ -52,7 +52,20 @@ The _flags_ parameter may have the following set, or be zero.
If _flags_ contains *TRACEFS_FL_RESET*, then it will clear the filters that
are currently set before applying the list of filters from _filters_. Otherwise,
the list of filters from _filters_ will be added to the current set of filters
-already enabled.
+already enabled. This flag is ignored if a previous call to
+tracefs_function_filter() had the same _instance_ and the
+*TRACEFS_FL_CONTINUE* flag was set.
+
+*TRACEFS_FL_CONTINUE* :
+If _flags_ contains *TRACEFS_FL_CONTINUE*, then the filters will not take
+effect after a successful call to tracefs_function_filter(). This allows for
+multiple calls to tracefs_function_filter() to update the filter function.
+It can be called multiple times and add more filters. A call without this
+flag set will commit the changes before returning (if the filters passed in
+successfully matched). A tracefs_function_filter() call after one that had
+the *TRACEFS_FL_CONTINUE* flag set for the same instance will ignore the
+*TRACEFS_FL_RESET* flag, as the reset flag is only applicable for the first
+filters to be added before committing.
RETURN VALUE
------------
@@ -70,13 +83,13 @@ EXAMPLE
#define INST "dummy"
-const char *filters[] = { "run_init_process", "try_to_run_init_process", "dummy1", NULL };
+static const char *filters[] = { "run_init_process", "try_to_run_init_process", "dummy1", NULL };
+static const char *all[] = { "*", NULL };
int main(int argc, char *argv[])
{
struct tracefs_instance *inst = tracefs_instance_create(INST);
const char **errs = NULL;
- int flags = TRACEFS_FL_RESET;
int ret;
int i = 0;
@@ -84,15 +97,24 @@ int main(int argc, char *argv[])
/* Error creating new trace instance */
}
- ret = tracefs_function_filter(inst, filters, NULL, flags, &errs);
+ ret = tracefs_function_filter(inst, filters, NULL,
+ TRACEFS_FL_RESET | TRACEF_FL_CONTINUE,
+ &errs);
if (ret < 0 && errs) {
while (errs[i])
printf("%s\n", errs[i++]);
}
+ free(errs);
+
+ ret = tracefs_function_filter(inst, all, "ext4", 0, &errs);
+ if (ret < 0) {
+ printf("Failed to set filters for ext4\n");
+ /* Force the function to commit previous filters */
+ tracefs_function_filter(inst, NULL, NULL, 0, &errs);
+ }
tracefs_instance_destroy(inst);
- free(errs);
return 0;
}
--
diff --git a/include/tracefs-local.h b/include/tracefs-local.h
index 9c18218cd916..73ec113fdb20 100644
--- a/include/tracefs-local.h
+++ b/include/tracefs-local.h
@@ -18,6 +18,7 @@ struct tracefs_instance {
char *trace_dir;
char *name;
int flags;
+ int ftrace_filter_fd;
};
/* Can be overridden */
diff --git a/include/tracefs.h b/include/tracefs.h
index 5193d46f41f5..f4775e938f69 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -148,9 +148,11 @@ const char *tracefs_option_name(enum tracefs_option_id id);
/*
* RESET - Reset on opening filter file (O_TRUNC)
+ * CONTINUE - Do not close filter file on return.
*/
enum {
TRACEFS_FL_RESET = (1 << 0),
+ TRACEFS_FL_CONTINUE = (1 << 1),
};
int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c
index a02c839f2079..bf2fabf111ea 100644
--- a/src/tracefs-instance.c
+++ b/src/tracefs-instance.c
@@ -43,6 +43,8 @@ static struct tracefs_instance *instance_alloc(const char *trace_dir, const char
goto error;
}
+ instance->ftrace_filter_fd = -1;
+
return instance;
error:
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 7e191e207867..862db5caa20e 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -23,6 +23,8 @@
#define TRACE_FILTER "set_ftrace_filter"
#define TRACE_FILTER_LIST "available_filter_functions"
+/* File descriptor for Top level set_ftrace_filter */
+static int ftrace_filter_fd = -1;
static pthread_mutex_t filter_lock = PTHREAD_MUTEX_INITIALIZER;
static const char * const options_map[] = {
@@ -851,6 +853,12 @@ static int write_func_list(int fd, struct func_list *list)
* before applying the @filters. This flag is ignored
* if this function is called again when the previous
* call had TRACEFS_FL_CONTINUE set.
+ * TRACEFS_FL_CONTINUE - will keep the filter file open on return.
+ * The filter is updated on closing of the filter file.
+ * With this flag set, the file is not closed, and more filters
+ * may be added before they take effect. The last call of this
+ * function must be called without this flag for the filter
+ * to take effect.
*
* returns -x on filter errors (where x is number of failed filter
* srtings) and if @errs is not NULL will be an allocated string array
@@ -869,13 +877,26 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
struct func_list *func_list = NULL;
char *ftrace_filter_path;
bool reset = flags & TRACEFS_FL_RESET;
+ bool cont = flags & TRACEFS_FL_CONTINUE;
int open_flags;
int ret = 1;
- int fd;
+ int *fd;
pthread_mutex_lock(&filter_lock);
- if (!filters)
+ if (instance)
+ fd = &instance->ftrace_filter_fd;
+ else
+ fd = &ftrace_filter_fd;
+
+ if (!filters) {
+ /* OK to call without filters if this is closing the opened file */
+ if (!cont && *fd >= 0) {
+ ret = 0;
+ close(*fd);
+ *fd = -1;
+ }
goto out;
+ }
func_filters = make_func_filters(filters);
if (!func_filters)
@@ -896,16 +917,20 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
open_flags = reset ? O_TRUNC : O_APPEND;
- fd = open(ftrace_filter_path, O_WRONLY | open_flags);
+ if (*fd < 0)
+ *fd = open(ftrace_filter_path, O_WRONLY | open_flags);
tracefs_put_tracing_file(ftrace_filter_path);
- if (fd < 0)
+ if (*fd < 0)
goto out_free;
- ret = write_func_list(fd, func_list);
+ ret = write_func_list(*fd, func_list);
if (ret > 0)
- ret = controlled_write(fd, func_filters, module, errs);
+ ret = controlled_write(*fd, func_filters, module, errs);
- close(fd);
+ if (!cont) {
+ close(*fd);
+ *fd = -1;
+ }
out_free:
free_func_list(func_list);
--
2.30.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC][PATCH 14/13 v2] libtracefs: Just past one filter in for tracefs_function_filter()
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
` (12 preceding siblings ...)
2021-03-30 0:51 ` [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter() Steven Rostedt
@ 2021-03-30 1:03 ` Steven Rostedt
2021-03-30 14:31 ` Tzvetomir Stoyanov
13 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 1:03 UTC (permalink / raw)
To: linux-trace-devel; +Cc: Sameeruddin shaik
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
[
Now that we have a way to call tracefs_function_filter() more than
once without committing the filter, I think it now makes sense to go
back to Tzvetomir's original proposal of calling this function once
per filter and not pass in an array. This greatly simplifies the
code, and makes the errs parameter obsolete. I played a little with
this interface, and I think it makes a lot of sense.
Comments?
]
With the new CONTINUE flag, it is inconsistent to have an array of filters
but only one module. Change the API to pass in a single filter, and remove
the errs parameter. Also change the return value to be 0 on success, 1 if
it failed before starting to write to the filter file, and -1 if it failed
after starting to write to the filter file.
Also, set errno appropriately, where if the filter fails EINVAL is set.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Documentation/libtracefs-function-filter.txt | 105 ++++++----
include/tracefs.h | 4 +-
src/tracefs-tools.c | 208 +++++--------------
3 files changed, 109 insertions(+), 208 deletions(-)
diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
index 1ac8a06..5631ff7 100644
--- a/Documentation/libtracefs-function-filter.txt
+++ b/Documentation/libtracefs-function-filter.txt
@@ -11,37 +11,31 @@ SYNOPSIS
--
*#include <tracefs.h>*
-int *tracefs_function_filter*(struct tracefs_instance pass:[*]_instance_, const char pass:[*]pass:[*]_filters_, const char pass:[*]_module_, int _flags_, const char pass:[*]pass:[*]pass:[*]_errs_);
+int *tracefs_function_filter*(struct tracefs_instance pass:[*]_instance_, const char pass:[*]_filter_, const char pass:[*]_module_, int _flags_);
--
DESCRIPTION
-----------
-This function can be used to limit the Linux kernel functions that were
+This function can be used to limit the Linux kernel functions that would be
traced by the function and function-graph tracers
-It will take
-_instance_ , that can be NULL for the top level tracing.
-_filters_, which is an array of the strings that represent a list of filters that should
-be applied to define what functions are to be traced and The array must end
-with a NULL pointer.
-_module_ , name of the module to be traced (or NULL for all functions),
+It will take an
+_instance_ , that can be NULL for the top level tracing,
+_filter_, a string that represents a filter that should
+be applied to define what functions are to be traced,
+_module_, to limit the filtering on a specific module (or NULL to filter on all functions),
_flags_ which holds control knobs on how the filters will be handled (see *FLAGS*)
-section below,
-_errs_, is a pointer to an array of strings, which will be allocated if
-any of filters fail to match any available function, If _errs_ is NULL, it will
-be ignored.
+section below.
-A filter in the array of _filters_ may be either a straight match of a
-function, a glob or regex(3). a glob is where 'pass:[*]' matches zero or more
+The _filter may be either a straight match of a
+function, a glob or regex(3). A glob is where 'pass:[*]' matches zero or more
characters, '?' will match zero or one character, and '.' only matches a
-period. If the filter is determined to be a regex (where it contains
-anything other than alpha numeric characters, or '.', 'pass:[*]', '?') the filter
+period. If the _filter_ is determined to be a regex (where it contains
+anything other than alpha numeric characters, or '.', 'pass:[*]', '?') the _filter_
will be processed as a regex(3) following the rules of regex(3), and '.' is
not a period, but will match any one character. To force a regular
-expression, either prefix the filter with a '^' or append it with a '$' as
-all filters will act as complete matches of functions anyway.
-
-returns 0 on success, 1 or -x (where x is an integer) on error.
+expression, either prefix _filter_ with a '^' or append it with a '$' as
+the _filter_ does complete matches of the functions anyway.
FLAGS
-----
@@ -50,18 +44,19 @@ The _flags_ parameter may have the following set, or be zero.
*TRACEFS_FL_RESET* :
If _flags_ contains *TRACEFS_FL_RESET*, then it will clear the filters that
-are currently set before applying the list of filters from _filters_. Otherwise,
-the list of filters from _filters_ will be added to the current set of filters
-already enabled. This flag is ignored if a previous call to
-tracefs_function_filter() had the same _instance_ and the
+are currently set before applying _filter_. Otherwise, _filter_ is added to
+the current set of filters already enabled. This flag is ignored if a
+previous call to tracefs_function_filter() had the same _instance_ and the
*TRACEFS_FL_CONTINUE* flag was set.
*TRACEFS_FL_CONTINUE* :
-If _flags_ contains *TRACEFS_FL_CONTINUE*, then the filters will not take
+If _flags_ contains *TRACEFS_FL_CONTINUE*, then _filter_ will not take
effect after a successful call to tracefs_function_filter(). This allows for
-multiple calls to tracefs_function_filter() to update the filter function.
-It can be called multiple times and add more filters. A call without this
-flag set will commit the changes before returning (if the filters passed in
+multiple calls to tracefs_function_filter() to update the filter function
+and then a single call (one without the *TRACEFS_FL_CONTINUE* flag set) to
+commit all the filters.
+It can be called multiple times to add more filters. A call without this
+flag set will commit the changes before returning (if the _filter_ passed in
successfully matched). A tracefs_function_filter() call after one that had
the *TRACEFS_FL_CONTINUE* flag set for the same instance will ignore the
*TRACEFS_FL_RESET* flag, as the reset flag is only applicable for the first
@@ -69,49 +64,67 @@ filters to be added before committing.
RETURN VALUE
------------
-return 0 on success, if there is error, it will return 1 for general errors or
-negative number -x(x denotes number of failed filters), if there are any failed filters.
+Returns 0 on success. If the there is an error but the filtering was not
+started, then 1 is returned. If filtering was started but an error occurs,
+then -1 is returned. The state of the filtering may be in an unknown state.
+
+If *TRACEFS_FL_CONTINUE* was set, and 0 or -1 was returned, then another call
+to tracefs_function_filter() must be done without *TRACEFS_FL_CONTINUE* set
+in order to commit (and close) the filtering.
+
+ERRORS
+------
-In case of negative return value, errs have to be checked and must be freed
-using the free()
+*tracefs_function_filter*() can fail with the following errors:
+
+*EINVAL* The filter is invalid or did not match any functions.
+
+Other errors may also happen caused by internal system calls.
EXAMPLE
-------
[source,c]
--
+#include <stdio.h>
+#include <errno.h>
#include <tracefs.h>
#define INST "dummy"
static const char *filters[] = { "run_init_process", "try_to_run_init_process", "dummy1", NULL };
-static const char *all[] = { "*", NULL };
int main(int argc, char *argv[])
{
struct tracefs_instance *inst = tracefs_instance_create(INST);
- const char **errs = NULL;
int ret;
- int i = 0;
+ int i;
if (!inst) {
/* Error creating new trace instance */
}
- ret = tracefs_function_filter(inst, filters, NULL,
- TRACEFS_FL_RESET | TRACEF_FL_CONTINUE,
- &errs);
-
- if (ret < 0 && errs) {
- while (errs[i])
- printf("%s\n", errs[i++]);
+ for (i = 0; filters[i]; i++) {
+ /*
+ * Note, only the first call does something
+ * with TRACEFS_FL_RESET. It is ignored in the following
+ * calls.
+ */
+ ret = tracefs_function_filter(inst, filters[i], NULL,
+ TRACEFS_FL_RESET | TRACEFS_FL_CONTINUE);
+
+ if (ret) {
+ if (errno == EINVAL)
+ printf("Filter %s did not match\n", filters[i]);
+ else
+ printf("Failed writing %s\n", filters[i]);
+ }
}
- free(errs);
- ret = tracefs_function_filter(inst, all, "ext4", 0, &errs);
- if (ret < 0) {
+ ret = tracefs_function_filter(inst, "*", "ext4", 0);
+ if (ret) {
printf("Failed to set filters for ext4\n");
/* Force the function to commit previous filters */
- tracefs_function_filter(inst, NULL, NULL, 0, &errs);
+ tracefs_function_filter(inst, NULL, NULL, 0);
}
tracefs_instance_destroy(inst);
diff --git a/include/tracefs.h b/include/tracefs.h
index f4775e9..0d98c10 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -155,6 +155,6 @@ enum {
TRACEFS_FL_CONTINUE = (1 << 1),
};
-int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
- const char *module, unsigned int flags, const char ***errs);
+int tracefs_function_filter(struct tracefs_instance *instance, const char *filter,
+ const char *module, unsigned int flags);
#endif /* _TRACE_FS_H */
diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
index 862db5c..165a9db 100644
--- a/src/tracefs-tools.c
+++ b/src/tracefs-tools.c
@@ -396,33 +396,6 @@ void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_opt
options->mask &= ~(1ULL << (id - 1));
}
-static void add_errors(const char ***errs, const char *filter, int ret)
-{
- const char **e;
-
- if (!errs)
- return;
-
- /* Negative is passed in */
- ret = -ret;
- e = *errs;
-
- /* If this previously failed to allocate stop processing */
- if (!e && ret)
- return;
-
- /* Add 2, one for the new entry, and one for NULL */
- e = realloc(e, sizeof(*e) * (ret + 2));
- if (!e) {
- free(*errs);
- *errs = NULL;
- return;
- }
- e[ret] = filter;
- e[ret + 1] = NULL;
- *errs = e;
-}
-
struct func_list {
struct func_list *next;
unsigned int start;
@@ -584,7 +557,7 @@ enum match_type {
FILTER_WRITE,
};
-static int match_filters(int fd, struct func_filter *func_filters,
+static int match_filters(int fd, struct func_filter *func_filter,
const char *module, struct func_list **func_list,
enum match_type type)
{
@@ -595,7 +568,6 @@ static int match_filters(int fd, struct func_filter *func_filters,
int index = 0;
int ret = 1;
int mlen;
- int i;
path = tracefs_get_tracing_file(TRACE_FILTER_LIST);
if (!path)
@@ -614,7 +586,6 @@ static int match_filters(int fd, struct func_filter *func_filters,
char *saveptr = NULL;
char *tok, *mtok;
int len = strlen(line);
- bool first = true;
if (line[len - 1] == '\n')
line[len - 1] = '\0';
@@ -634,30 +605,16 @@ static int match_filters(int fd, struct func_filter *func_filters,
}
switch (type) {
case FILTER_CHECK:
- /* Check, checks a list of filters */
- for (i = 0; func_filters[i].filter; i++) {
- /*
- * If a match was found, still need to
- * check if other filters would match
- * to make sure that all filters have a
- * match, as some filters may overlap.
- */
- if (!first && func_filters[i].set)
- continue;
- if (match(tok, &func_filters[i])) {
- func_filters[i].set = true;
- if (first) {
- first = false;
- ret = add_func(&func_list, index);
- if (ret)
- goto out;
- }
- }
+ if (match(tok, func_filter)) {
+ func_filter->set = true;
+ ret = add_func(&func_list, index);
+ if (ret)
+ goto out;
}
break;
case FILTER_WRITE:
/* Writes only have one filter */
- if (match(tok, func_filters)) {
+ if (match(tok, func_filter)) {
ret = write_filter(fd, tok, module);
if (ret)
goto out;
@@ -676,25 +633,11 @@ static int match_filters(int fd, struct func_filter *func_filters,
return ret;
}
-static int check_available_filters(struct func_filter *func_filters,
- const char *module, const char ***errs,
+static int check_available_filters(struct func_filter *func_filter,
+ const char *module,
struct func_list **func_list)
{
- int ret;
- int i;
-
- ret = match_filters(-1, func_filters, module, func_list, FILTER_CHECK);
- /* Return here if success or non filter error */
- if (ret >= 0)
- return ret;
-
- /* Failed on filter, set the errors */
- ret = 0;
- for (i = 0; func_filters[i].filter; i++) {
- if (!func_filters[i].set)
- add_errors(errs, func_filters[i].filter, ret--);
- }
- return ret;
+ return match_filters(-1, func_filter, module, func_list, FILTER_CHECK);
}
static int set_regex_filter(int fd, struct func_filter *func_filter,
@@ -703,31 +646,17 @@ static int set_regex_filter(int fd, struct func_filter *func_filter,
return match_filters(fd, func_filter, module, NULL, FILTER_WRITE);
}
-static int controlled_write(int fd, struct func_filter *func_filters,
- const char *module, const char ***errs)
+static int controlled_write(int fd, struct func_filter *func_filter,
+ const char *module)
{
- int ret = 0;
- int i;
+ const char *filter = func_filter->filter;
+ int ret;
+
+ if (func_filter->is_regex)
+ ret = set_regex_filter(fd, func_filter, module);
+ else
+ ret = write_filter(fd, filter, module);
- for (i = 0; func_filters[i].filter; i++) {
- const char *filter = func_filters[i].filter;
- int r;
-
- if (func_filters[i].is_regex)
- r = set_regex_filter(fd, &func_filters[i], module);
- else
- r = write_filter(fd, filter, module);
- if (r > 0) {
- /* Not filter error */
- if (errs) {
- free(*errs);
- *errs = NULL;
- }
- return 1;
- }
- if (r < 0)
- add_errors(errs, filter, ret--);
- }
return ret;
}
@@ -754,43 +683,6 @@ static int init_func_filter(struct func_filter *func_filter, const char *filter)
return 0;
}
-static void free_func_filters(struct func_filter *func_filters)
-{
- int i;
-
- if (!func_filters)
- return;
-
- for (i = 0; func_filters[i].filter; i++) {
- regfree(&func_filters[i].re);
- }
-}
-
-static struct func_filter *make_func_filters(const char **filters)
-{
- struct func_filter *func_filters = NULL;
- int i;
-
- for (i = 0; filters[i]; i++)
- ;
-
- if (!i)
- return NULL;
-
- func_filters = calloc(i + 1, sizeof(*func_filters));
- if (!func_filters)
- return NULL;
-
- for (i = 0; filters[i]; i++) {
- if (init_func_filter(&func_filters[i], filters[i]) < 0)
- goto out_err;
- }
- return func_filters;
- out_err:
- free_func_filters(func_filters);
- return NULL;
-}
-
static int write_number(int fd, unsigned int start, unsigned int end)
{
char buf[64];
@@ -835,22 +727,19 @@ static int write_func_list(int fd, struct func_list *list)
}
/**
- * tracefs_function_filter - write to set_ftrace_filter file to trace
- * particular functions
- * @instance: ftrace instance, can be NULL for top tracing instance
- * @filters: An array of function names ending with a NULL pointer
- * @module: Module to be traced
+ * tracefs_function_filter - filter the functions that are traced
+ * @instance: ftrace instance, can be NULL for top tracing instance.
+ * @filter: The filter to filter what functions are to be traced
+ * @module: Module to be traced or NULL if all functions are to be examined.
* @flags: flags on modifying the filter file
- * @errs: A pointer to array of constant strings that will be allocated
- * on negative return of this function, pointing to the filters that
- * failed.May be NULL, in which case this field will be ignored.
*
- * The @filters is an array of strings, where each string will be used
- * to set a function or functions to be traced.
+ * @filter may be a full function name, a glob, or a regex. It will be
+ * considered a regex, if there's any characters that are not normally in
+ * function names or "*" or "?" for a glob.
*
* @flags:
* TRACEFS_FL_RESET - will clear the functions in the filter file
- * before applying the @filters. This flag is ignored
+ * before applying the @filter. This flag is ignored
* if this function is called again when the previous
* call had TRACEFS_FL_CONTINUE set.
* TRACEFS_FL_CONTINUE - will keep the filter file open on return.
@@ -860,20 +749,16 @@ static int write_func_list(int fd, struct func_list *list)
* function must be called without this flag for the filter
* to take effect.
*
- * returns -x on filter errors (where x is number of failed filter
- * srtings) and if @errs is not NULL will be an allocated string array
- * pointing to the strings in @filters that failed and must be freed
- * with free().
- *
- * returns 1 on general errors not realted to setting the filter.
- * @errs is not set even if supplied.
- *
- * return 0 on success and @errs is not set.
+ * Returns 0 on success, 1 if there was an error but the filtering has not
+ * yet started, -1 if there was an error but the filtering has started.
+ * If -1 is returned and TRACEFS_FL_CONTINUE was set, then this function
+ * needs to be called again without the TRACEFS_FL_CONTINUE flag to commit
+ * the changes and close the filter file.
*/
-int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
- const char *module, unsigned int flags, const char ***errs)
+int tracefs_function_filter(struct tracefs_instance *instance, const char *filter,
+ const char *module, unsigned int flags)
{
- struct func_filter *func_filters;
+ struct func_filter func_filter;
struct func_list *func_list = NULL;
char *ftrace_filter_path;
bool reset = flags & TRACEFS_FL_RESET;
@@ -888,9 +773,16 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
else
fd = &ftrace_filter_fd;
- if (!filters) {
+ /*
+ * Set EINVAL on no matching filter. But errno may still be modified
+ * on another type of failure (allocation or opening a file).
+ */
+ errno = EINVAL;
+
+ if (!filter) {
/* OK to call without filters if this is closing the opened file */
if (!cont && *fd >= 0) {
+ errno = 0;
ret = 0;
close(*fd);
*fd = -1;
@@ -898,15 +790,10 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
goto out;
}
- func_filters = make_func_filters(filters);
- if (!func_filters)
+ if (init_func_filter(&func_filter, filter) < 0)
goto out;
- /* Make sure errs is NULL to start with, realloc() depends on it. */
- if (errs)
- *errs = NULL;
-
- ret = check_available_filters(func_filters, module, errs, &func_list);
+ ret = check_available_filters(&func_filter, module, &func_list);
if (ret)
goto out_free;
@@ -923,9 +810,11 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
if (*fd < 0)
goto out_free;
+ errno = 0;
+
ret = write_func_list(*fd, func_list);
if (ret > 0)
- ret = controlled_write(*fd, func_filters, module, errs);
+ ret = controlled_write(*fd, &func_filter, module);
if (!cont) {
close(*fd);
@@ -934,7 +823,6 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
out_free:
free_func_list(func_list);
- free_func_filters(func_filters);
out:
pthread_mutex_unlock(&filter_lock);
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 10/13 v2] libtracefs: Pass in reset via flags to tracefs_function_filter()
2021-03-30 0:51 ` [PATCH 10/13 v2] libtracefs: Pass in reset via flags to tracefs_function_filter() Steven Rostedt
@ 2021-03-30 14:29 ` Tzvetomir Stoyanov
2021-03-30 14:53 ` Steven Rostedt
0 siblings, 1 reply; 27+ messages in thread
From: Tzvetomir Stoyanov @ 2021-03-30 14:29 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Linux Trace Devel, Sameeruddin shaik
On Tue, Mar 30, 2021 at 3:54 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> Instead of having a separate boolean to tell tracefs_function_filter() to
> reset the file upon opening, make it a flag instead. This way other
> booleans can be passed into the function without having to extend the
> parameters.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> Documentation/libtracefs-function-filter.txt | 26 ++++++++++++++------
> include/tracefs.h | 10 +++++++-
> src/tracefs-tools.c | 19 ++++++++------
> 3 files changed, 38 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
> index 88aa3b923d54..5b55a72727c8 100644
> --- a/Documentation/libtracefs-function-filter.txt
> +++ b/Documentation/libtracefs-function-filter.txt
> @@ -11,7 +11,7 @@ SYNOPSIS
> --
> *#include <tracefs.h>*
>
> -int *tracefs_function_filter*(struct tracefs_instance pass:[*]_instance_, const char pass:[*]pass:[*]_filters_, const char pass:[*]_module_, bool _reset_, const char pass:[*]pass:[*]pass:[*]_errs_);
> +int *tracefs_function_filter*(struct tracefs_instance pass:[*]_instance_, const char pass:[*]pass:[*]_filters_, const char pass:[*]_module_, int _flags_, const char pass:[*]pass:[*]pass:[*]_errs_);
> --
>
> DESCRIPTION
> @@ -25,18 +25,17 @@ _filters_, which is an array of the strings that represent a list of filters tha
> be applied to define what functions are to be traced and The array must end
> with a NULL pointer.
> _module_ , name of the module to be traced (or NULL for all functions),
> -_reset_ if set will clear the current set of filters and then apply the
> -filter list, otherwise the list of filters are added to the current set of
> -filters,
> +_flags_ which holds control knobs on how the filters will be handled (see *FLAGS*)
> +section below,
> _errs_, is a pointer to an array of strings, which will be allocated if
> any of filters fail to match any available function, If _errs_ is NULL, it will
> be ignored.
>
> A filter in the array of _filters_ may be either a straight match of a
> -function, a glob or regex(3). a glob is where '*' matches zero or more
> +function, a glob or regex(3). a glob is where 'pass:[*]' matches zero or more
> characters, '?' will match zero or one character, and '.' only matches a
> period. If the filter is determined to be a regex (where it contains
> -anything other than alpha numeric characters, or '.', '*', '?') the filter
> +anything other than alpha numeric characters, or '.', 'pass:[*]', '?') the filter
> will be processed as a regex(3) following the rules of regex(3), and '.' is
> not a period, but will match any one character. To force a regular
> expression, either prefix the filter with a '^' or append it with a '$' as
> @@ -44,6 +43,17 @@ all filters will act as complete matches of functions anyway.
>
> returns 0 on success, 1 or -x (where x is an integer) on error.
>
> +FLAGS
> +-----
> +
> +The _flags_ parameter may have the following set, or be zero.
> +
> +*TRACEFS_FL_RESET* :
> +If _flags_ contains *TRACEFS_FL_RESET*, then it will clear the filters that
> +are currently set before applying the list of filters from _filters_. Otherwise,
> +the list of filters from _filters_ will be added to the current set of filters
> +already enabled.
> +
> RETURN VALUE
> ------------
> return 0 on success, if there is error, it will return 1 for general errors or
> @@ -66,7 +76,7 @@ int main(int argc, char *argv[])
> {
> struct tracefs_instance *inst = tracefs_instance_create(INST);
> const char **errs = NULL;
> - bool reset = 1;
> + int flags = TRACEFS_FL_RESET;
> int ret;
> int i = 0;
>
> @@ -74,7 +84,7 @@ int main(int argc, char *argv[])
> /* Error creating new trace instance */
> }
>
> - ret = tracefs_function_filter(inst, filters, NULL, reset, &errs);
> + ret = tracefs_function_filter(inst, filters, NULL, flags, &errs);
>
> if (ret < 0 && errs) {
> while (errs[i])
> diff --git a/include/tracefs.h b/include/tracefs.h
> index 9477fdb14c5a..5193d46f41f5 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -145,6 +145,14 @@ bool tracefs_option_is_enabled(struct tracefs_instance *instance, enum tracefs_o
> int tracefs_option_enable(struct tracefs_instance *instance, enum tracefs_option_id id);
> int tracefs_option_diasble(struct tracefs_instance *instance, enum tracefs_option_id id);
> const char *tracefs_option_name(enum tracefs_option_id id);
> +
> +/*
> + * RESET - Reset on opening filter file (O_TRUNC)
> + */
> +enum {
> + TRACEFS_FL_RESET = (1 << 0),
> +};
> +
> int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
> - const char *module, bool reset, const char ***errs);
> + const char *module, unsigned int flags, const char ***errs);
> #endif /* _TRACE_FS_H */
> diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
> index 8f2130948fe0..6d03b4856a63 100644
> --- a/src/tracefs-tools.c
> +++ b/src/tracefs-tools.c
> @@ -835,7 +835,7 @@ static int write_func_list(int fd, struct func_list *list)
> * @instance: ftrace instance, can be NULL for top tracing instance
> * @filters: An array of function names ending with a NULL pointer
> * @module: Module to be traced
> - * @reset: set to true to reset the file before applying the filter
> + * @flags: flags on modifying the filter file
> * @errs: A pointer to array of constant strings that will be allocated
> * on negative return of this function, pointing to the filters that
> * failed.May be NULL, in which case this field will be ignored.
> @@ -843,9 +843,11 @@ static int write_func_list(int fd, struct func_list *list)
> * The @filters is an array of strings, where each string will be used
> * to set a function or functions to be traced.
> *
> - * If @reset is true, then all functions in the filter are cleared
> - * before adding functions from @filters. Otherwise, the functions set
> - * by @filters will be appended to the filter file
> + * @flags:
> + * TRACEFS_FL_RESET - will clear the functions in the filter file
> + * before applying the @filters. This flag is ignored
> + * if this function is called again when the previous
> + * call had TRACEFS_FL_CONTINUE set.
> *
> * returns -x on filter errors (where x is number of failed filter
> * srtings) and if @errs is not NULL will be an allocated string array
> @@ -858,12 +860,13 @@ static int write_func_list(int fd, struct func_list *list)
> * return 0 on success and @errs is not set.
> */
> int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
> - const char *module, bool reset, const char ***errs)
> + const char *module, unsigned int flags, const char ***errs)
> {
> struct func_filter *func_filters;
> struct func_list *func_list = NULL;
> char *ftrace_filter_path;
> - int flags;
> + bool reset = flags & TRACEFS_FL_RESET;
> + int open_flags;
> int ret;
> int fd;
>
> @@ -887,9 +890,9 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
> if (!ftrace_filter_path)
> goto out_free;
>
> - flags = reset ? O_TRUNC : O_APPEND;
> + open_flags = reset ? O_TRUNC : O_APPEND;
>
> - fd = open(ftrace_filter_path, O_WRONLY | flags);
> + fd = open(ftrace_filter_path, O_WRONLY | open_flags);
> tracefs_put_tracing_file(ftrace_filter_path);
> if (fd < 0)
> goto out_free;
> --
> 2.30.1
>
>
It will be useful to allow calling the API with RESET flag set and no
filters, just to reset the current filters without adding new:
tracefs_function_filter(NULL, NULL, NULL,TRACEFS_FL_RESET, NULL);
could be a valid call to reset filters in the top trace instance.
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter()
2021-03-30 0:51 ` [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter() Steven Rostedt
@ 2021-03-30 14:29 ` Tzvetomir Stoyanov
2021-03-30 14:52 ` Steven Rostedt
0 siblings, 1 reply; 27+ messages in thread
From: Tzvetomir Stoyanov @ 2021-03-30 14:29 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Linux Trace Devel, Sameeruddin shaik
On Tue, Mar 30, 2021 at 3:54 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> Add the TRACEFS_FL_CONTINUE flag to the tracefs_function_filter() API that
> will allow it to return without closing the set_ftrace_filter file. When
> the set_ftrace_filter file is closed, all the changes to it take place,
> but not before hand. In the case that multiple modules need to be set in
> one activation, the tracefs_function_filter() would need to be called
> multiple times without closing the file descriptor.
>
> Note, the next call to tracefs_function_filter() after it was called with
> the CONTINUE flag set, the RESET flag is ignored, as the RESET flag only
> takes effect on opening the file. The next call to
> tracefs_function_filter() after it was called with the CONTINUE flag (on
> the same instance) does not reopen the file, and thus will not reset the
> file.
I think it is better to not maintain a state in the API context. Let's
make it simple and allow the user to take care of the calling order
and flags.
If RESET is set - close the file (if it is opened already) and open
it with the TRUNC flag.
If CONTINUE is set - do not close the file.
Also, allow calling the API with no filters and any combination of
flags, just to have resting and committing currently written filters.
For example:
tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_RESET, NULL); //
Close the file (if it is opened already), open it with the TRUNC flag
and close it.
tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_CONTINUE, NULL);
// Open the file with APPEND (if it is not opened already) and do not
close it.
tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_RESET |
TRACEFS_FL_CONTINUE, NULL); // Close the file (if it is opened
already), open it with the TRUNC flag and do not close it.
>
> If the file is opened, calling tracefs_function_filter() with no filters
> and the continue flag not set, will simply close the set_ftrace_filter
> file.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> Documentation/libtracefs-function-filter.txt | 32 +++++++++++++---
> include/tracefs-local.h | 1 +
> include/tracefs.h | 2 +
> src/tracefs-instance.c | 2 +
> src/tracefs-tools.c | 39 ++++++++++++++++----
> 5 files changed, 64 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
> index 5b55a72727c8..1ac8a06961bf 100644
> --- a/Documentation/libtracefs-function-filter.txt
> +++ b/Documentation/libtracefs-function-filter.txt
> @@ -52,7 +52,20 @@ The _flags_ parameter may have the following set, or be zero.
> If _flags_ contains *TRACEFS_FL_RESET*, then it will clear the filters that
> are currently set before applying the list of filters from _filters_. Otherwise,
> the list of filters from _filters_ will be added to the current set of filters
> -already enabled.
> +already enabled. This flag is ignored if a previous call to
> +tracefs_function_filter() had the same _instance_ and the
> +*TRACEFS_FL_CONTINUE* flag was set.
> +
> +*TRACEFS_FL_CONTINUE* :
> +If _flags_ contains *TRACEFS_FL_CONTINUE*, then the filters will not take
> +effect after a successful call to tracefs_function_filter(). This allows for
> +multiple calls to tracefs_function_filter() to update the filter function.
> +It can be called multiple times and add more filters. A call without this
> +flag set will commit the changes before returning (if the filters passed in
> +successfully matched). A tracefs_function_filter() call after one that had
> +the *TRACEFS_FL_CONTINUE* flag set for the same instance will ignore the
> +*TRACEFS_FL_RESET* flag, as the reset flag is only applicable for the first
> +filters to be added before committing.
>
> RETURN VALUE
> ------------
> @@ -70,13 +83,13 @@ EXAMPLE
>
> #define INST "dummy"
>
> -const char *filters[] = { "run_init_process", "try_to_run_init_process", "dummy1", NULL };
> +static const char *filters[] = { "run_init_process", "try_to_run_init_process", "dummy1", NULL };
> +static const char *all[] = { "*", NULL };
>
> int main(int argc, char *argv[])
> {
> struct tracefs_instance *inst = tracefs_instance_create(INST);
> const char **errs = NULL;
> - int flags = TRACEFS_FL_RESET;
> int ret;
> int i = 0;
>
> @@ -84,15 +97,24 @@ int main(int argc, char *argv[])
> /* Error creating new trace instance */
> }
>
> - ret = tracefs_function_filter(inst, filters, NULL, flags, &errs);
> + ret = tracefs_function_filter(inst, filters, NULL,
> + TRACEFS_FL_RESET | TRACEF_FL_CONTINUE,
> + &errs);
>
> if (ret < 0 && errs) {
> while (errs[i])
> printf("%s\n", errs[i++]);
> }
> + free(errs);
> +
> + ret = tracefs_function_filter(inst, all, "ext4", 0, &errs);
> + if (ret < 0) {
> + printf("Failed to set filters for ext4\n");
> + /* Force the function to commit previous filters */
> + tracefs_function_filter(inst, NULL, NULL, 0, &errs);
> + }
>
> tracefs_instance_destroy(inst);
> - free(errs);
> return 0;
> }
> --
> diff --git a/include/tracefs-local.h b/include/tracefs-local.h
> index 9c18218cd916..73ec113fdb20 100644
> --- a/include/tracefs-local.h
> +++ b/include/tracefs-local.h
> @@ -18,6 +18,7 @@ struct tracefs_instance {
> char *trace_dir;
> char *name;
> int flags;
> + int ftrace_filter_fd;
> };
>
> /* Can be overridden */
> diff --git a/include/tracefs.h b/include/tracefs.h
> index 5193d46f41f5..f4775e938f69 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -148,9 +148,11 @@ const char *tracefs_option_name(enum tracefs_option_id id);
>
> /*
> * RESET - Reset on opening filter file (O_TRUNC)
> + * CONTINUE - Do not close filter file on return.
> */
> enum {
> TRACEFS_FL_RESET = (1 << 0),
> + TRACEFS_FL_CONTINUE = (1 << 1),
> };
>
> int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
> diff --git a/src/tracefs-instance.c b/src/tracefs-instance.c
> index a02c839f2079..bf2fabf111ea 100644
> --- a/src/tracefs-instance.c
> +++ b/src/tracefs-instance.c
> @@ -43,6 +43,8 @@ static struct tracefs_instance *instance_alloc(const char *trace_dir, const char
> goto error;
> }
>
> + instance->ftrace_filter_fd = -1;
> +
> return instance;
>
> error:
> diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
> index 7e191e207867..862db5caa20e 100644
> --- a/src/tracefs-tools.c
> +++ b/src/tracefs-tools.c
> @@ -23,6 +23,8 @@
> #define TRACE_FILTER "set_ftrace_filter"
> #define TRACE_FILTER_LIST "available_filter_functions"
>
> +/* File descriptor for Top level set_ftrace_filter */
> +static int ftrace_filter_fd = -1;
> static pthread_mutex_t filter_lock = PTHREAD_MUTEX_INITIALIZER;
>
> static const char * const options_map[] = {
> @@ -851,6 +853,12 @@ static int write_func_list(int fd, struct func_list *list)
> * before applying the @filters. This flag is ignored
> * if this function is called again when the previous
> * call had TRACEFS_FL_CONTINUE set.
> + * TRACEFS_FL_CONTINUE - will keep the filter file open on return.
> + * The filter is updated on closing of the filter file.
> + * With this flag set, the file is not closed, and more filters
> + * may be added before they take effect. The last call of this
> + * function must be called without this flag for the filter
> + * to take effect.
> *
> * returns -x on filter errors (where x is number of failed filter
> * srtings) and if @errs is not NULL will be an allocated string array
> @@ -869,13 +877,26 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
> struct func_list *func_list = NULL;
> char *ftrace_filter_path;
> bool reset = flags & TRACEFS_FL_RESET;
> + bool cont = flags & TRACEFS_FL_CONTINUE;
> int open_flags;
> int ret = 1;
> - int fd;
> + int *fd;
>
> pthread_mutex_lock(&filter_lock);
> - if (!filters)
> + if (instance)
> + fd = &instance->ftrace_filter_fd;
> + else
> + fd = &ftrace_filter_fd;
> +
> + if (!filters) {
> + /* OK to call without filters if this is closing the opened file */
> + if (!cont && *fd >= 0) {
> + ret = 0;
> + close(*fd);
> + *fd = -1;
> + }
> goto out;
> + }
>
> func_filters = make_func_filters(filters);
> if (!func_filters)
> @@ -896,16 +917,20 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
>
> open_flags = reset ? O_TRUNC : O_APPEND;
>
> - fd = open(ftrace_filter_path, O_WRONLY | open_flags);
> + if (*fd < 0)
> + *fd = open(ftrace_filter_path, O_WRONLY | open_flags);
> tracefs_put_tracing_file(ftrace_filter_path);
> - if (fd < 0)
> + if (*fd < 0)
> goto out_free;
>
> - ret = write_func_list(fd, func_list);
> + ret = write_func_list(*fd, func_list);
> if (ret > 0)
> - ret = controlled_write(fd, func_filters, module, errs);
> + ret = controlled_write(*fd, func_filters, module, errs);
>
> - close(fd);
> + if (!cont) {
> + close(*fd);
> + *fd = -1;
> + }
>
> out_free:
> free_func_list(func_list);
> --
> 2.30.1
>
>
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC][PATCH 14/13 v2] libtracefs: Just past one filter in for tracefs_function_filter()
2021-03-30 1:03 ` [RFC][PATCH 14/13 v2] libtracefs: Just past one filter in for tracefs_function_filter() Steven Rostedt
@ 2021-03-30 14:31 ` Tzvetomir Stoyanov
0 siblings, 0 replies; 27+ messages in thread
From: Tzvetomir Stoyanov @ 2021-03-30 14:31 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Linux Trace Devel, Sameeruddin shaik
On Tue, Mar 30, 2021 at 4:04 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> [
> Now that we have a way to call tracefs_function_filter() more than
> once without committing the filter, I think it now makes sense to go
> back to Tzvetomir's original proposal of calling this function once
> per filter and not pass in an array. This greatly simplifies the
> code, and makes the errs parameter obsolete. I played a little with
> this interface, and I think it makes a lot of sense.
>
> Comments?
> ]
I like the idea to simplify the logic.
>
> With the new CONTINUE flag, it is inconsistent to have an array of filters
> but only one module. Change the API to pass in a single filter, and remove
> the errs parameter. Also change the return value to be 0 on success, 1 if
> it failed before starting to write to the filter file, and -1 if it failed
> after starting to write to the filter file.
>
> Also, set errno appropriately, where if the filter fails EINVAL is set.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> Documentation/libtracefs-function-filter.txt | 105 ++++++----
> include/tracefs.h | 4 +-
> src/tracefs-tools.c | 208 +++++--------------
> 3 files changed, 109 insertions(+), 208 deletions(-)
>
> diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
> index 1ac8a06..5631ff7 100644
> --- a/Documentation/libtracefs-function-filter.txt
> +++ b/Documentation/libtracefs-function-filter.txt
> @@ -11,37 +11,31 @@ SYNOPSIS
> --
> *#include <tracefs.h>*
>
> -int *tracefs_function_filter*(struct tracefs_instance pass:[*]_instance_, const char pass:[*]pass:[*]_filters_, const char pass:[*]_module_, int _flags_, const char pass:[*]pass:[*]pass:[*]_errs_);
> +int *tracefs_function_filter*(struct tracefs_instance pass:[*]_instance_, const char pass:[*]_filter_, const char pass:[*]_module_, int _flags_);
> --
>
> DESCRIPTION
> -----------
> -This function can be used to limit the Linux kernel functions that were
> +This function can be used to limit the Linux kernel functions that would be
> traced by the function and function-graph tracers
>
> -It will take
> -_instance_ , that can be NULL for the top level tracing.
> -_filters_, which is an array of the strings that represent a list of filters that should
> -be applied to define what functions are to be traced and The array must end
> -with a NULL pointer.
> -_module_ , name of the module to be traced (or NULL for all functions),
> +It will take an
> +_instance_ , that can be NULL for the top level tracing,
> +_filter_, a string that represents a filter that should
> +be applied to define what functions are to be traced,
> +_module_, to limit the filtering on a specific module (or NULL to filter on all functions),
> _flags_ which holds control knobs on how the filters will be handled (see *FLAGS*)
> -section below,
> -_errs_, is a pointer to an array of strings, which will be allocated if
> -any of filters fail to match any available function, If _errs_ is NULL, it will
> -be ignored.
> +section below.
>
> -A filter in the array of _filters_ may be either a straight match of a
> -function, a glob or regex(3). a glob is where 'pass:[*]' matches zero or more
> +The _filter may be either a straight match of a
> +function, a glob or regex(3). A glob is where 'pass:[*]' matches zero or more
> characters, '?' will match zero or one character, and '.' only matches a
> -period. If the filter is determined to be a regex (where it contains
> -anything other than alpha numeric characters, or '.', 'pass:[*]', '?') the filter
> +period. If the _filter_ is determined to be a regex (where it contains
> +anything other than alpha numeric characters, or '.', 'pass:[*]', '?') the _filter_
> will be processed as a regex(3) following the rules of regex(3), and '.' is
> not a period, but will match any one character. To force a regular
> -expression, either prefix the filter with a '^' or append it with a '$' as
> -all filters will act as complete matches of functions anyway.
> -
> -returns 0 on success, 1 or -x (where x is an integer) on error.
> +expression, either prefix _filter_ with a '^' or append it with a '$' as
> +the _filter_ does complete matches of the functions anyway.
>
> FLAGS
> -----
> @@ -50,18 +44,19 @@ The _flags_ parameter may have the following set, or be zero.
>
> *TRACEFS_FL_RESET* :
> If _flags_ contains *TRACEFS_FL_RESET*, then it will clear the filters that
> -are currently set before applying the list of filters from _filters_. Otherwise,
> -the list of filters from _filters_ will be added to the current set of filters
> -already enabled. This flag is ignored if a previous call to
> -tracefs_function_filter() had the same _instance_ and the
> +are currently set before applying _filter_. Otherwise, _filter_ is added to
> +the current set of filters already enabled. This flag is ignored if a
> +previous call to tracefs_function_filter() had the same _instance_ and the
> *TRACEFS_FL_CONTINUE* flag was set.
>
> *TRACEFS_FL_CONTINUE* :
> -If _flags_ contains *TRACEFS_FL_CONTINUE*, then the filters will not take
> +If _flags_ contains *TRACEFS_FL_CONTINUE*, then _filter_ will not take
> effect after a successful call to tracefs_function_filter(). This allows for
> -multiple calls to tracefs_function_filter() to update the filter function.
> -It can be called multiple times and add more filters. A call without this
> -flag set will commit the changes before returning (if the filters passed in
> +multiple calls to tracefs_function_filter() to update the filter function
> +and then a single call (one without the *TRACEFS_FL_CONTINUE* flag set) to
> +commit all the filters.
> +It can be called multiple times to add more filters. A call without this
> +flag set will commit the changes before returning (if the _filter_ passed in
> successfully matched). A tracefs_function_filter() call after one that had
> the *TRACEFS_FL_CONTINUE* flag set for the same instance will ignore the
> *TRACEFS_FL_RESET* flag, as the reset flag is only applicable for the first
> @@ -69,49 +64,67 @@ filters to be added before committing.
>
> RETURN VALUE
> ------------
> -return 0 on success, if there is error, it will return 1 for general errors or
> -negative number -x(x denotes number of failed filters), if there are any failed filters.
> +Returns 0 on success. If the there is an error but the filtering was not
> +started, then 1 is returned. If filtering was started but an error occurs,
> +then -1 is returned. The state of the filtering may be in an unknown state.
> +
> +If *TRACEFS_FL_CONTINUE* was set, and 0 or -1 was returned, then another call
> +to tracefs_function_filter() must be done without *TRACEFS_FL_CONTINUE* set
> +in order to commit (and close) the filtering.
> +
> +ERRORS
> +------
>
> -In case of negative return value, errs have to be checked and must be freed
> -using the free()
> +*tracefs_function_filter*() can fail with the following errors:
> +
> +*EINVAL* The filter is invalid or did not match any functions.
> +
> +Other errors may also happen caused by internal system calls.
>
> EXAMPLE
> -------
> [source,c]
> --
> +#include <stdio.h>
> +#include <errno.h>
> #include <tracefs.h>
>
> #define INST "dummy"
>
> static const char *filters[] = { "run_init_process", "try_to_run_init_process", "dummy1", NULL };
> -static const char *all[] = { "*", NULL };
>
> int main(int argc, char *argv[])
> {
> struct tracefs_instance *inst = tracefs_instance_create(INST);
> - const char **errs = NULL;
> int ret;
> - int i = 0;
> + int i;
>
> if (!inst) {
> /* Error creating new trace instance */
> }
>
> - ret = tracefs_function_filter(inst, filters, NULL,
> - TRACEFS_FL_RESET | TRACEF_FL_CONTINUE,
> - &errs);
> -
> - if (ret < 0 && errs) {
> - while (errs[i])
> - printf("%s\n", errs[i++]);
> + for (i = 0; filters[i]; i++) {
> + /*
> + * Note, only the first call does something
> + * with TRACEFS_FL_RESET. It is ignored in the following
> + * calls.
> + */
> + ret = tracefs_function_filter(inst, filters[i], NULL,
> + TRACEFS_FL_RESET | TRACEFS_FL_CONTINUE);
> +
> + if (ret) {
> + if (errno == EINVAL)
> + printf("Filter %s did not match\n", filters[i]);
> + else
> + printf("Failed writing %s\n", filters[i]);
> + }
> }
> - free(errs);
>
> - ret = tracefs_function_filter(inst, all, "ext4", 0, &errs);
> - if (ret < 0) {
> + ret = tracefs_function_filter(inst, "*", "ext4", 0);
> + if (ret) {
> printf("Failed to set filters for ext4\n");
> /* Force the function to commit previous filters */
> - tracefs_function_filter(inst, NULL, NULL, 0, &errs);
> + tracefs_function_filter(inst, NULL, NULL, 0);
> }
>
> tracefs_instance_destroy(inst);
> diff --git a/include/tracefs.h b/include/tracefs.h
> index f4775e9..0d98c10 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -155,6 +155,6 @@ enum {
> TRACEFS_FL_CONTINUE = (1 << 1),
> };
>
> -int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
> - const char *module, unsigned int flags, const char ***errs);
> +int tracefs_function_filter(struct tracefs_instance *instance, const char *filter,
> + const char *module, unsigned int flags);
> #endif /* _TRACE_FS_H */
> diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
> index 862db5c..165a9db 100644
> --- a/src/tracefs-tools.c
> +++ b/src/tracefs-tools.c
> @@ -396,33 +396,6 @@ void tracefs_option_clear(struct tracefs_options_mask *options, enum tracefs_opt
> options->mask &= ~(1ULL << (id - 1));
> }
>
> -static void add_errors(const char ***errs, const char *filter, int ret)
> -{
> - const char **e;
> -
> - if (!errs)
> - return;
> -
> - /* Negative is passed in */
> - ret = -ret;
> - e = *errs;
> -
> - /* If this previously failed to allocate stop processing */
> - if (!e && ret)
> - return;
> -
> - /* Add 2, one for the new entry, and one for NULL */
> - e = realloc(e, sizeof(*e) * (ret + 2));
> - if (!e) {
> - free(*errs);
> - *errs = NULL;
> - return;
> - }
> - e[ret] = filter;
> - e[ret + 1] = NULL;
> - *errs = e;
> -}
> -
> struct func_list {
> struct func_list *next;
> unsigned int start;
> @@ -584,7 +557,7 @@ enum match_type {
> FILTER_WRITE,
> };
>
> -static int match_filters(int fd, struct func_filter *func_filters,
> +static int match_filters(int fd, struct func_filter *func_filter,
> const char *module, struct func_list **func_list,
> enum match_type type)
> {
> @@ -595,7 +568,6 @@ static int match_filters(int fd, struct func_filter *func_filters,
> int index = 0;
> int ret = 1;
> int mlen;
> - int i;
>
> path = tracefs_get_tracing_file(TRACE_FILTER_LIST);
> if (!path)
> @@ -614,7 +586,6 @@ static int match_filters(int fd, struct func_filter *func_filters,
> char *saveptr = NULL;
> char *tok, *mtok;
> int len = strlen(line);
> - bool first = true;
>
> if (line[len - 1] == '\n')
> line[len - 1] = '\0';
> @@ -634,30 +605,16 @@ static int match_filters(int fd, struct func_filter *func_filters,
> }
> switch (type) {
> case FILTER_CHECK:
> - /* Check, checks a list of filters */
> - for (i = 0; func_filters[i].filter; i++) {
> - /*
> - * If a match was found, still need to
> - * check if other filters would match
> - * to make sure that all filters have a
> - * match, as some filters may overlap.
> - */
> - if (!first && func_filters[i].set)
> - continue;
> - if (match(tok, &func_filters[i])) {
> - func_filters[i].set = true;
> - if (first) {
> - first = false;
> - ret = add_func(&func_list, index);
> - if (ret)
> - goto out;
> - }
> - }
> + if (match(tok, func_filter)) {
> + func_filter->set = true;
> + ret = add_func(&func_list, index);
> + if (ret)
> + goto out;
> }
> break;
> case FILTER_WRITE:
> /* Writes only have one filter */
> - if (match(tok, func_filters)) {
> + if (match(tok, func_filter)) {
> ret = write_filter(fd, tok, module);
> if (ret)
> goto out;
> @@ -676,25 +633,11 @@ static int match_filters(int fd, struct func_filter *func_filters,
> return ret;
> }
>
> -static int check_available_filters(struct func_filter *func_filters,
> - const char *module, const char ***errs,
> +static int check_available_filters(struct func_filter *func_filter,
> + const char *module,
> struct func_list **func_list)
> {
> - int ret;
> - int i;
> -
> - ret = match_filters(-1, func_filters, module, func_list, FILTER_CHECK);
> - /* Return here if success or non filter error */
> - if (ret >= 0)
> - return ret;
> -
> - /* Failed on filter, set the errors */
> - ret = 0;
> - for (i = 0; func_filters[i].filter; i++) {
> - if (!func_filters[i].set)
> - add_errors(errs, func_filters[i].filter, ret--);
> - }
> - return ret;
> + return match_filters(-1, func_filter, module, func_list, FILTER_CHECK);
> }
>
> static int set_regex_filter(int fd, struct func_filter *func_filter,
> @@ -703,31 +646,17 @@ static int set_regex_filter(int fd, struct func_filter *func_filter,
> return match_filters(fd, func_filter, module, NULL, FILTER_WRITE);
> }
>
> -static int controlled_write(int fd, struct func_filter *func_filters,
> - const char *module, const char ***errs)
> +static int controlled_write(int fd, struct func_filter *func_filter,
> + const char *module)
> {
> - int ret = 0;
> - int i;
> + const char *filter = func_filter->filter;
> + int ret;
> +
> + if (func_filter->is_regex)
> + ret = set_regex_filter(fd, func_filter, module);
> + else
> + ret = write_filter(fd, filter, module);
>
> - for (i = 0; func_filters[i].filter; i++) {
> - const char *filter = func_filters[i].filter;
> - int r;
> -
> - if (func_filters[i].is_regex)
> - r = set_regex_filter(fd, &func_filters[i], module);
> - else
> - r = write_filter(fd, filter, module);
> - if (r > 0) {
> - /* Not filter error */
> - if (errs) {
> - free(*errs);
> - *errs = NULL;
> - }
> - return 1;
> - }
> - if (r < 0)
> - add_errors(errs, filter, ret--);
> - }
> return ret;
> }
>
> @@ -754,43 +683,6 @@ static int init_func_filter(struct func_filter *func_filter, const char *filter)
> return 0;
> }
>
> -static void free_func_filters(struct func_filter *func_filters)
> -{
> - int i;
> -
> - if (!func_filters)
> - return;
> -
> - for (i = 0; func_filters[i].filter; i++) {
> - regfree(&func_filters[i].re);
> - }
> -}
> -
> -static struct func_filter *make_func_filters(const char **filters)
> -{
> - struct func_filter *func_filters = NULL;
> - int i;
> -
> - for (i = 0; filters[i]; i++)
> - ;
> -
> - if (!i)
> - return NULL;
> -
> - func_filters = calloc(i + 1, sizeof(*func_filters));
> - if (!func_filters)
> - return NULL;
> -
> - for (i = 0; filters[i]; i++) {
> - if (init_func_filter(&func_filters[i], filters[i]) < 0)
> - goto out_err;
> - }
> - return func_filters;
> - out_err:
> - free_func_filters(func_filters);
> - return NULL;
> -}
> -
> static int write_number(int fd, unsigned int start, unsigned int end)
> {
> char buf[64];
> @@ -835,22 +727,19 @@ static int write_func_list(int fd, struct func_list *list)
> }
>
> /**
> - * tracefs_function_filter - write to set_ftrace_filter file to trace
> - * particular functions
> - * @instance: ftrace instance, can be NULL for top tracing instance
> - * @filters: An array of function names ending with a NULL pointer
> - * @module: Module to be traced
> + * tracefs_function_filter - filter the functions that are traced
> + * @instance: ftrace instance, can be NULL for top tracing instance.
> + * @filter: The filter to filter what functions are to be traced
> + * @module: Module to be traced or NULL if all functions are to be examined.
> * @flags: flags on modifying the filter file
> - * @errs: A pointer to array of constant strings that will be allocated
> - * on negative return of this function, pointing to the filters that
> - * failed.May be NULL, in which case this field will be ignored.
> *
> - * The @filters is an array of strings, where each string will be used
> - * to set a function or functions to be traced.
> + * @filter may be a full function name, a glob, or a regex. It will be
> + * considered a regex, if there's any characters that are not normally in
> + * function names or "*" or "?" for a glob.
> *
> * @flags:
> * TRACEFS_FL_RESET - will clear the functions in the filter file
> - * before applying the @filters. This flag is ignored
> + * before applying the @filter. This flag is ignored
> * if this function is called again when the previous
> * call had TRACEFS_FL_CONTINUE set.
> * TRACEFS_FL_CONTINUE - will keep the filter file open on return.
> @@ -860,20 +749,16 @@ static int write_func_list(int fd, struct func_list *list)
> * function must be called without this flag for the filter
> * to take effect.
> *
> - * returns -x on filter errors (where x is number of failed filter
> - * srtings) and if @errs is not NULL will be an allocated string array
> - * pointing to the strings in @filters that failed and must be freed
> - * with free().
> - *
> - * returns 1 on general errors not realted to setting the filter.
> - * @errs is not set even if supplied.
> - *
> - * return 0 on success and @errs is not set.
> + * Returns 0 on success, 1 if there was an error but the filtering has not
> + * yet started, -1 if there was an error but the filtering has started.
> + * If -1 is returned and TRACEFS_FL_CONTINUE was set, then this function
> + * needs to be called again without the TRACEFS_FL_CONTINUE flag to commit
> + * the changes and close the filter file.
> */
> -int tracefs_function_filter(struct tracefs_instance *instance, const char **filters,
> - const char *module, unsigned int flags, const char ***errs)
> +int tracefs_function_filter(struct tracefs_instance *instance, const char *filter,
> + const char *module, unsigned int flags)
> {
> - struct func_filter *func_filters;
> + struct func_filter func_filter;
> struct func_list *func_list = NULL;
> char *ftrace_filter_path;
> bool reset = flags & TRACEFS_FL_RESET;
> @@ -888,9 +773,16 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
> else
> fd = &ftrace_filter_fd;
>
> - if (!filters) {
> + /*
> + * Set EINVAL on no matching filter. But errno may still be modified
> + * on another type of failure (allocation or opening a file).
> + */
> + errno = EINVAL;
> +
> + if (!filter) {
> /* OK to call without filters if this is closing the opened file */
> if (!cont && *fd >= 0) {
> + errno = 0;
> ret = 0;
> close(*fd);
> *fd = -1;
> @@ -898,15 +790,10 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
> goto out;
> }
>
> - func_filters = make_func_filters(filters);
> - if (!func_filters)
> + if (init_func_filter(&func_filter, filter) < 0)
> goto out;
>
> - /* Make sure errs is NULL to start with, realloc() depends on it. */
> - if (errs)
> - *errs = NULL;
> -
> - ret = check_available_filters(func_filters, module, errs, &func_list);
> + ret = check_available_filters(&func_filter, module, &func_list);
> if (ret)
> goto out_free;
>
> @@ -923,9 +810,11 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
> if (*fd < 0)
> goto out_free;
>
> + errno = 0;
> +
> ret = write_func_list(*fd, func_list);
> if (ret > 0)
> - ret = controlled_write(*fd, func_filters, module, errs);
> + ret = controlled_write(*fd, &func_filter, module);
>
> if (!cont) {
> close(*fd);
> @@ -934,7 +823,6 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
>
> out_free:
> free_func_list(func_list);
> - free_func_filters(func_filters);
> out:
> pthread_mutex_unlock(&filter_lock);
>
> --
> 2.29.2
>
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter()
2021-03-30 14:29 ` Tzvetomir Stoyanov
@ 2021-03-30 14:52 ` Steven Rostedt
2021-03-30 15:14 ` Steven Rostedt
2021-03-30 15:32 ` Tzvetomir Stoyanov
0 siblings, 2 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 14:52 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: Linux Trace Devel, Sameeruddin shaik
On Tue, 30 Mar 2021 17:29:57 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
> On Tue, Mar 30, 2021 at 3:54 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >
> > Add the TRACEFS_FL_CONTINUE flag to the tracefs_function_filter() API that
> > will allow it to return without closing the set_ftrace_filter file. When
> > the set_ftrace_filter file is closed, all the changes to it take place,
> > but not before hand. In the case that multiple modules need to be set in
> > one activation, the tracefs_function_filter() would need to be called
> > multiple times without closing the file descriptor.
> >
> > Note, the next call to tracefs_function_filter() after it was called with
> > the CONTINUE flag set, the RESET flag is ignored, as the RESET flag only
> > takes effect on opening the file. The next call to
> > tracefs_function_filter() after it was called with the CONTINUE flag (on
> > the same instance) does not reopen the file, and thus will not reset the
> > file.
>
> I think it is better to not maintain a state in the API context. Let's
> make it simple and allow the user to take care of the calling order
> and flags.
> If RESET is set - close the file (if it is opened already) and open
> it with the TRUNC flag.
In the use cases I've played with, it was simple to do:
for (i = 0; filters[i]; i++)
tracefs_function_filter(instance, filters[i], module,
TRACEFS_FL_RESET | TRACEFS_FL_CONTINUE);
And not worry about keeping track of the RESET flag. This is the reason I
did it this way. Otherwise I would need to be:
int reset = TRACEFS_FL_RESET;
for (i = 0; filters[i]; i++) {
tracefs_function_filter(instance, filters[i], module,
reset | TRACEFS_FL_CONTINUE);
reset = 0;
}
Or something else. But I'm looking to simplify the most common case.
And closing the file can cause issues. If we were to go this way, I would
make it that if the file is open and RESET is set, it would return an error
EBUSY.
But no, closing the file just because RESET is set is dangerous.
> If CONTINUE is set - do not close the file.
> Also, allow calling the API with no filters and any combination of
> flags, just to have resting and committing currently written filters.
> For example:
> tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_RESET, NULL); //
> Close the file (if it is opened already), open it with the TRUNC flag
> and close it.
> tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_CONTINUE, NULL);
> // Open the file with APPEND (if it is not opened already) and do not
> close it.
> tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_RESET |
> TRACEFS_FL_CONTINUE, NULL); // Close the file (if it is opened
> already), open it with the TRUNC flag and do not close it.
Yes, I had already planned on adding a patch to allow NULL filter when
these flags are set. I just didn't get there yet.
-- Steve
>
>
> >
> > If the file is opened, calling tracefs_function_filter() with no filters
> > and the continue flag not set, will simply close the set_ftrace_filter
> > file.
> >
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > ---
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 10/13 v2] libtracefs: Pass in reset via flags to tracefs_function_filter()
2021-03-30 14:29 ` Tzvetomir Stoyanov
@ 2021-03-30 14:53 ` Steven Rostedt
0 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 14:53 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: Linux Trace Devel, Sameeruddin shaik
On Tue, 30 Mar 2021 17:29:35 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
> It will be useful to allow calling the API with RESET flag set and no
> filters, just to reset the current filters without adding new:
> tracefs_function_filter(NULL, NULL, NULL,TRACEFS_FL_RESET, NULL);
> could be a valid call to reset filters in the top trace instance.
Agreed. I planned on adding a patch to do just that.
-- Steve
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter()
2021-03-30 14:52 ` Steven Rostedt
@ 2021-03-30 15:14 ` Steven Rostedt
2021-03-30 15:32 ` Tzvetomir Stoyanov
1 sibling, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 15:14 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: Linux Trace Devel, Sameeruddin shaik
On Tue, 30 Mar 2021 10:52:50 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> And closing the file can cause issues. If we were to go this way, I would
> make it that if the file is open and RESET is set, it would return an error
> EBUSY.
>
> But no, closing the file just because RESET is set is dangerous.
So, I'm going to keep the patches going as is, and build on top of them
with updates.
Thinking about this some more, I believe I'll go with returning 1 with
errno = EBUSY if RESET is passed in and the file descriptor is opened.
-- Steve
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter()
2021-03-30 14:52 ` Steven Rostedt
2021-03-30 15:14 ` Steven Rostedt
@ 2021-03-30 15:32 ` Tzvetomir Stoyanov
2021-03-30 16:03 ` Steven Rostedt
1 sibling, 1 reply; 27+ messages in thread
From: Tzvetomir Stoyanov @ 2021-03-30 15:32 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Linux Trace Devel, Sameeruddin shaik
On Tue, Mar 30, 2021 at 5:52 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 30 Mar 2021 17:29:57 +0300
> Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
>
> > On Tue, Mar 30, 2021 at 3:54 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> > >
> > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > >
> > > Add the TRACEFS_FL_CONTINUE flag to the tracefs_function_filter() API that
> > > will allow it to return without closing the set_ftrace_filter file. When
> > > the set_ftrace_filter file is closed, all the changes to it take place,
> > > but not before hand. In the case that multiple modules need to be set in
> > > one activation, the tracefs_function_filter() would need to be called
> > > multiple times without closing the file descriptor.
> > >
> > > Note, the next call to tracefs_function_filter() after it was called with
> > > the CONTINUE flag set, the RESET flag is ignored, as the RESET flag only
> > > takes effect on opening the file. The next call to
> > > tracefs_function_filter() after it was called with the CONTINUE flag (on
> > > the same instance) does not reopen the file, and thus will not reset the
> > > file.
> >
> > I think it is better to not maintain a state in the API context. Let's
> > make it simple and allow the user to take care of the calling order
> > and flags.
> > If RESET is set - close the file (if it is opened already) and open
> > it with the TRUNC flag.
>
> In the use cases I've played with, it was simple to do:
>
> for (i = 0; filters[i]; i++)
> tracefs_function_filter(instance, filters[i], module,
> TRACEFS_FL_RESET | TRACEFS_FL_CONTINUE);
>
If the user takes care of the state, instead of the library, this use
case could look like:
tracefs_function_filter(instance, NULL, NULL, TRACEFS_FL_RESET |
TRACEFS_FL_CONTINUE);
/* or add a wrapper tracefs_function_filter_reset() */
for (i = 0; filters[i]; i++)
tracefs_function_filter(instance, filters[i], module,
TRACEFS_FL_CONTINUE);
tracefs_function_filter(instance, NULL, NULL, 0);
/* or add a wrapper tracefs_function_filter_commit() */
> And not worry about keeping track of the RESET flag. This is the reason I
> did it this way. Otherwise I would need to be:
>
> int reset = TRACEFS_FL_RESET;
> for (i = 0; filters[i]; i++) {
> tracefs_function_filter(instance, filters[i], module,
> reset | TRACEFS_FL_CONTINUE);
> reset = 0;
> }
>
> Or something else. But I'm looking to simplify the most common case.
>
> And closing the file can cause issues. If we were to go this way, I would
> make it that if the file is open and RESET is set, it would return an error
> EBUSY.
>
> But no, closing the file just because RESET is set is dangerous.
>
> > If CONTINUE is set - do not close the file.
> > Also, allow calling the API with no filters and any combination of
> > flags, just to have resting and committing currently written filters.
> > For example:
> > tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_RESET, NULL); //
> > Close the file (if it is opened already), open it with the TRUNC flag
> > and close it.
> > tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_CONTINUE, NULL);
> > // Open the file with APPEND (if it is not opened already) and do not
> > close it.
> > tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_RESET |
> > TRACEFS_FL_CONTINUE, NULL); // Close the file (if it is opened
> > already), open it with the TRUNC flag and do not close it.
>
> Yes, I had already planned on adding a patch to allow NULL filter when
> these flags are set. I just didn't get there yet.
>
> -- Steve
>
> >
> >
> > >
> > > If the file is opened, calling tracefs_function_filter() with no filters
> > > and the continue flag not set, will simply close the set_ftrace_filter
> > > file.
> > >
> > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > > ---
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter()
2021-03-30 15:32 ` Tzvetomir Stoyanov
@ 2021-03-30 16:03 ` Steven Rostedt
0 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-03-30 16:03 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: Linux Trace Devel, Sameeruddin shaik
On Tue, 30 Mar 2021 18:32:25 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
> On Tue, Mar 30, 2021 at 5:52 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Tue, 30 Mar 2021 17:29:57 +0300
> > Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
> >
> > > On Tue, Mar 30, 2021 at 3:54 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> > > >
> > > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > > >
> > > > Add the TRACEFS_FL_CONTINUE flag to the tracefs_function_filter() API that
> > > > will allow it to return without closing the set_ftrace_filter file. When
> > > > the set_ftrace_filter file is closed, all the changes to it take place,
> > > > but not before hand. In the case that multiple modules need to be set in
> > > > one activation, the tracefs_function_filter() would need to be called
> > > > multiple times without closing the file descriptor.
> > > >
> > > > Note, the next call to tracefs_function_filter() after it was called with
> > > > the CONTINUE flag set, the RESET flag is ignored, as the RESET flag only
> > > > takes effect on opening the file. The next call to
> > > > tracefs_function_filter() after it was called with the CONTINUE flag (on
> > > > the same instance) does not reopen the file, and thus will not reset the
> > > > file.
> > >
> > > I think it is better to not maintain a state in the API context. Let's
> > > make it simple and allow the user to take care of the calling order
> > > and flags.
> > > If RESET is set - close the file (if it is opened already) and open
> > > it with the TRUNC flag.
> >
> > In the use cases I've played with, it was simple to do:
> >
> > for (i = 0; filters[i]; i++)
> > tracefs_function_filter(instance, filters[i], module,
> > TRACEFS_FL_RESET | TRACEFS_FL_CONTINUE);
> >
>
> If the user takes care of the state, instead of the library, this use
> case could look like:
>
> tracefs_function_filter(instance, NULL, NULL, TRACEFS_FL_RESET |
> TRACEFS_FL_CONTINUE);
> /* or add a wrapper tracefs_function_filter_reset() */
>
> for (i = 0; filters[i]; i++)
> tracefs_function_filter(instance, filters[i], module,
> TRACEFS_FL_CONTINUE);
>
> tracefs_function_filter(instance, NULL, NULL, 0);
> /* or add a wrapper tracefs_function_filter_commit() */
Yes, I thought the same, and as I replied in another email, I'm going to
make it that RESET and the file already opened will simply fail, and let
the user decide what to do.
On a side note, please sign off your email with some type of sig (or crop
the rest of the email), to let me know that you are done commenting, so I
don't continue scrolling through the reply to see if you have anything else
to say.
Thanks!
-- Steve
>
>
> > And not worry about keeping track of the RESET flag. This is the reason I
> > did it this way. Otherwise I would need to be:
> >
> > int reset = TRACEFS_FL_RESET;
> > for (i = 0; filters[i]; i++) {
> > tracefs_function_filter(instance, filters[i], module,
> > reset | TRACEFS_FL_CONTINUE);
> > reset = 0;
> > }
> >
> > Or something else. But I'm looking to simplify the most common case.
> >
> > And closing the file can cause issues. If we were to go this way, I would
> > make it that if the file is open and RESET is set, it would return an error
> > EBUSY.
> >
> > But no, closing the file just because RESET is set is dangerous.
> >
> > > If CONTINUE is set - do not close the file.
> > > Also, allow calling the API with no filters and any combination of
> > > flags, just to have resting and committing currently written filters.
> > > For example:
> > > tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_RESET, NULL); //
> > > Close the file (if it is opened already), open it with the TRUNC flag
> > > and close it.
> > > tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_CONTINUE, NULL);
> > > // Open the file with APPEND (if it is not opened already) and do not
> > > close it.
> > > tracefs_function_filter(NULL, NULL, NULL, TRACEFS_FL_RESET |
> > > TRACEFS_FL_CONTINUE, NULL); // Close the file (if it is opened
> > > already), open it with the TRUNC flag and do not close it.
> >
> > Yes, I had already planned on adding a patch to allow NULL filter when
> > these flags are set. I just didn't get there yet.
> >
> > -- Steve
> >
> > >
> > >
> > > >
> > > > If the file is opened, calling tracefs_function_filter() with no filters
> > > > and the continue flag not set, will simply close the set_ftrace_filter
> > > > file.
> > > >
> > > > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > > > ---
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 08/13 v2] libtracefs: Allow for setting filters with regex expressions
2021-04-01 16:33 ` sameeruddin shaik
@ 2021-03-31 16:39 ` Steven Rostedt
2021-04-02 1:59 ` sameeruddin shaik
0 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2021-03-31 16:39 UTC (permalink / raw)
To: sameeruddin shaik; +Cc: linux-trace-devel
On Thu, 1 Apr 2021 22:03:02 +0530
sameeruddin shaik <sameeruddin.shaik8@gmail.com> wrote:
> hi steve,
>
> On 30/03/21 6:21 am, Steven Rostedt wrote:
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >
> > All for full "regex(3)" processing of setting functions in the
> > set_ftrace_filter file. Check if the filter passed in is just a glob
> > expression that the kernel can process, or if it is a regex that should look
> > at the available_filter_functions list instead.
> >
> > If it is a regex, it will read the available_filter_functions and write in
> > each function as it finds it.
> >
> > Link: https://lore.kernel.org/linux-trace-devel/20210323013225.451281989@goodmis.org
> >
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > ---
> > Documentation/libtracefs-function-filter.txt | 10 ++
> > src/tracefs-tools.c | 139 ++++++++++++++++---
> > 2 files changed, 128 insertions(+), 21 deletions(-)
> >
> > diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
> > index c0c89f372c21..88aa3b923d54 100644
> > --- a/Documentation/libtracefs-function-filter.txt
> > +++ b/Documentation/libtracefs-function-filter.txt
> > @@ -32,6 +32,16 @@ _errs_, is a pointer to an array of strings, which will be allocated if
> > any of filters fail to match any available function, If _errs_ is NULL, it will
> > be ignored.
> >
> > +A filter in the array of _filters_ may be either a straight match of a
> > +function, a glob or regex(3). a glob is where '*' matches zero or more
> > +characters, '?' will match zero or one character, and '.' only matches a
> > +period. If the filter is determined to be a regex (where it contains
> > +anything other than alpha numeric characters, or '.', '*', '?') the filter
> > +will be processed as a regex(3) following the rules of regex(3), and '.' is
> > +not a period, but will match any one character. To force a regular
> > +expression, either prefix the filter with a '^' or append it with a '$' as
> > +all filters will act as complete matches of functions anyway.
> > +
>
> if we give the filter as regex "^ext4*$" from user side, ideally it
> should match the ext4 filter functions, if i am not wrong, its not
> matching any filter in the available_filter_functions
If you add the "^" and/or "$" it makes it into a regex. The above means
that the filter will match "ext", "ext4", "ext44", "ext4444444"
Because "*" means zero or more of the previous character. So, unless
there's a function that matches one of the above, it wont match anything
else.
If you left off the "^" and "$" then it would be a glob, where "*" means
zero or more of any character. But if you want the same in regex, you need
to use:
"^ext4.*$"
>
> is this expected behaviour?
>
> if we give the filter as glob "ext4*" from userside, its making the
> regex and matching the ext4 filter functions in the
> available_filter_functions.
Yes, because the conversion changes the glob "ext4*" into "^ext4.*$".
Also, for email etiquette, please sign you last comment, or cut the rest of
the reply. That way I don't have to scroll the rest of the email to know if
there's anything else you commented on.
-- Steve
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 08/13 v2] libtracefs: Allow for setting filters with regex expressions
2021-04-02 1:59 ` sameeruddin shaik
@ 2021-04-01 2:41 ` Steven Rostedt
0 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2021-04-01 2:41 UTC (permalink / raw)
To: sameeruddin shaik; +Cc: linux-trace-devel
On Fri, 2 Apr 2021 07:29:59 +0530
sameeruddin shaik <sameeruddin.shaik8@gmail.com> wrote:
> >
> > If you left off the "^" and "$" then it would be a glob, where "*" means
> > zero or more of any character. But if you want the same in regex, you need
> > to use:
> >
> > "^ext4.*$"
>
> IF we use the regex in filters, running time of the program is
> increasing drastically.
Only if the kernel doesn't support indexing, then it would take less
than a second.
>
> lets say,
>
> if we give the kernel glob as a filter, its getting converted to regex
> and running time of program is
>
> 5 secs, in other case where we use regex, its taking 80 secs to complete.
What example did you use? And yes, regex would do the matching in user
space and globs would be processed in the kernel (unless indexing is
available, in which case it uses regex for everything). That's because
the globs in the kernel were created to speed up the selection of
multiple functions. The regex would pass in one function that matched
at a time, and that's more of a O(n^2) algorithm.
This is why the function supports both globs and regex.
For simple regex, we could optimize it to use a glob, if indexing is
not supported. I mentioned this to Tzvetomir before and we agreed that
this optimization can be done with later patches in the future, as
modern kernels (5.1 and beyond) support indexing.
-- Steve
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 08/13 v2] libtracefs: Allow for setting filters with regex expressions
2021-03-30 0:51 ` [PATCH 08/13 v2] libtracefs: Allow for setting filters with regex expressions Steven Rostedt
@ 2021-04-01 16:33 ` sameeruddin shaik
2021-03-31 16:39 ` Steven Rostedt
0 siblings, 1 reply; 27+ messages in thread
From: sameeruddin shaik @ 2021-04-01 16:33 UTC (permalink / raw)
To: Steven Rostedt, linux-trace-devel
hi steve,
On 30/03/21 6:21 am, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> All for full "regex(3)" processing of setting functions in the
> set_ftrace_filter file. Check if the filter passed in is just a glob
> expression that the kernel can process, or if it is a regex that should look
> at the available_filter_functions list instead.
>
> If it is a regex, it will read the available_filter_functions and write in
> each function as it finds it.
>
> Link: https://lore.kernel.org/linux-trace-devel/20210323013225.451281989@goodmis.org
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> Documentation/libtracefs-function-filter.txt | 10 ++
> src/tracefs-tools.c | 139 ++++++++++++++++---
> 2 files changed, 128 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
> index c0c89f372c21..88aa3b923d54 100644
> --- a/Documentation/libtracefs-function-filter.txt
> +++ b/Documentation/libtracefs-function-filter.txt
> @@ -32,6 +32,16 @@ _errs_, is a pointer to an array of strings, which will be allocated if
> any of filters fail to match any available function, If _errs_ is NULL, it will
> be ignored.
>
> +A filter in the array of _filters_ may be either a straight match of a
> +function, a glob or regex(3). a glob is where '*' matches zero or more
> +characters, '?' will match zero or one character, and '.' only matches a
> +period. If the filter is determined to be a regex (where it contains
> +anything other than alpha numeric characters, or '.', '*', '?') the filter
> +will be processed as a regex(3) following the rules of regex(3), and '.' is
> +not a period, but will match any one character. To force a regular
> +expression, either prefix the filter with a '^' or append it with a '$' as
> +all filters will act as complete matches of functions anyway.
> +
if we give the filter as regex "^ext4*$" from user side, ideally it
should match the ext4 filter functions, if i am not wrong, its not
matching any filter in the available_filter_functions
is this expected behaviour?
if we give the filter as glob "ext4*" from userside, its making the
regex and matching the ext4 filter functions in the
available_filter_functions.
> returns 0 on success, 1 or -x (where x is an integer) on error.
>
> RETURN VALUE
> diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c
> index 470502b07f7d..d1a448459c6f 100644
> --- a/src/tracefs-tools.c
> +++ b/src/tracefs-tools.c
> @@ -18,8 +18,9 @@
> #include "tracefs.h"
> #include "tracefs-local.h"
>
> -#define TRACE_CTRL "tracing_on"
> -#define TRACE_FILTER "set_ftrace_filter"
> +#define TRACE_CTRL "tracing_on"
> +#define TRACE_FILTER "set_ftrace_filter"
> +#define TRACE_FILTER_LIST "available_filter_functions"
>
> static const char * const options_map[] = {
> "unknown",
> @@ -421,8 +422,53 @@ struct func_filter {
> const char *filter;
> regex_t re;
> bool set;
> + bool is_regex;
> };
>
> +static bool is_regex(const char *str)
> +{
> + int i;
> +
> + for (i = 0; str[i]; i++) {
> + switch (str[i]) {
> + case 'a' ... 'z':
> + case 'A'...'Z':
> + case '_':
> + case '0'...'9':
> + case '*':
> + case '.':
> + /* Dots can be part of a function name */
> + case '?':
> + continue;
> + default:
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static char *update_regex(const char *reg)
> +{
> + int len = strlen(reg);
> + char *str;
> +
> + if (reg[0] == '^' && reg[len - 1] == '$')
> + return strdup(reg);
> +
> + str = malloc(len + 3);
> + if (reg[0] == '^') {
> + strcpy(str, reg);
> + } else {
> + str[0] = '^';
> + strcpy(str + 1, reg);
> + len++; /* add ^ */
> + }
> + if (str[len - 1] != '$')
> + str[len++]= '$';
> + str[len] = '\0';
> + return str;
> +}
> +
> /*
> * Convert a glob into a regular expression.
> */
> @@ -488,8 +534,13 @@ static int write_filter(int fd, const char *filter, const char *module)
> return 0;
> }
>
> -static int check_available_filters(struct func_filter *func_filters,
> - const char *module, const char ***errs)
> +enum match_type {
> + FILTER_CHECK,
> + FILTER_WRITE,
> +};
> +
> +static int match_filters(int fd, struct func_filter *func_filters,
> + const char *module, enum match_type type)
> {
> char *line = NULL;
> size_t size = 0;
> @@ -499,7 +550,7 @@ static int check_available_filters(struct func_filter *func_filters,
> int mlen;
> int i;
>
> - path = tracefs_get_tracing_file("available_filter_functions");
> + path = tracefs_get_tracing_file(TRACE_FILTER_LIST);
> if (!path)
> return 1;
>
> @@ -530,39 +581,76 @@ static int check_available_filters(struct func_filter *func_filters,
> (mtok[mlen + 1] != ']'))
> goto next;
> }
> - for (i = 0; func_filters[i].filter; i++) {
> - if (match(tok, &func_filters[i]))
> - func_filters[i].set = true;
> + switch (type) {
> + case FILTER_CHECK:
> + /* Check, checks a list of filters */
> + for (i = 0; func_filters[i].filter; i++) {
> + if (match(tok, &func_filters[i]))
> + func_filters[i].set = true;
> + }
> + break;
> + case FILTER_WRITE:
> + /* Writes only have one filter */
> + if (match(tok, func_filters)) {
> + ret = write_filter(fd, tok, module);
> + if (ret)
> + goto out;
> + }
> + break;
> }
> next:
> free(line);
> line = NULL;
> len = 0;
> }
> + out:
> + free(line);
> fclose(fp);
>
> + return ret;
> +}
> +
> +static int check_available_filters(struct func_filter *func_filters,
> + const char *module, const char ***errs)
> +{
> + int ret;
> + int i;
> +
> + ret = match_filters(-1, func_filters, module, FILTER_CHECK);
> + /* Return here if success or non filter error */
> + if (ret >= 0)
> + return ret;
> +
> + /* Failed on filter, set the errors */
> ret = 0;
> for (i = 0; func_filters[i].filter; i++) {
> if (!func_filters[i].set)
> add_errors(errs, func_filters[i].filter, ret--);
> }
> -
> return ret;
> }
>
> -static int controlled_write(int fd, const char **filters,
> +static int set_regex_filter(int fd, struct func_filter *func_filter,
> + const char *module)
> +{
> + return match_filters(fd, func_filter, module, FILTER_WRITE);
> +}
> +
> +static int controlled_write(int fd, struct func_filter *func_filters,
> const char *module, const char ***errs)
> {
> int ret = 0;
> int i;
>
> - for (i = 0; filters[i]; i++) {
> + for (i = 0; func_filters[i].filter; i++) {
> + const char *filter = func_filters[i].filter;
> int r;
>
> - r = write_filter(fd, filters[i], module);
> - if (r < 0) {
> - add_errors(errs, filters[i], ret--);
> - } else if (r > 0) {
> + if (func_filters[i].is_regex)
> + r = set_regex_filter(fd, &func_filters[i], module);
> + else
> + r = write_filter(fd, filter, module);
> + if (r > 0) {
> /* Not filter error */
> if (errs) {
> free(*errs);
> @@ -570,6 +658,8 @@ static int controlled_write(int fd, const char **filters,
> }
> return 1;
> }
> + if (r < 0)
> + add_errors(errs, filter, ret--);
> }
> return ret;
> }
> @@ -579,7 +669,11 @@ static int init_func_filter(struct func_filter *func_filter, const char *filter)
> char *str;
> int ret;
>
> - str = make_regex(filter);
> + if (!(func_filter->is_regex = is_regex(filter)))
> + str = make_regex(filter);
> + else
> + str = update_regex(filter);
> +
> if (!str)
> return -1;
>
> @@ -679,24 +773,27 @@ int tracefs_function_filter(struct tracefs_instance *instance, const char **filt
> *errs = NULL;
>
> ret = check_available_filters(func_filters, module, errs);
> - free_func_filters(func_filters);
> if (ret)
> - return ret;
> + goto out_free;
>
> + ret = 1;
> ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER);
> if (!ftrace_filter_path)
> - return 1;
> + goto out_free;
>
> flags = reset ? O_TRUNC : O_APPEND;
>
> fd = open(ftrace_filter_path, O_WRONLY | flags);
> tracefs_put_tracing_file(ftrace_filter_path);
> if (fd < 0)
> - return 1;
> + goto out_free;
>
> - ret = controlled_write(fd, filters, module, errs);
> + ret = controlled_write(fd, func_filters, module, errs);
>
> close(fd);
>
> + out_free:
> + free_func_filters(func_filters);
> +
> return ret;
> }
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 08/13 v2] libtracefs: Allow for setting filters with regex expressions
2021-03-31 16:39 ` Steven Rostedt
@ 2021-04-02 1:59 ` sameeruddin shaik
2021-04-01 2:41 ` Steven Rostedt
0 siblings, 1 reply; 27+ messages in thread
From: sameeruddin shaik @ 2021-04-02 1:59 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel
hi steve,
On 31/03/21 10:09 pm, Steven Rostedt wrote:
> On Thu, 1 Apr 2021 22:03:02 +0530
> sameeruddin shaik <sameeruddin.shaik8@gmail.com> wrote:
>
>> hi steve,
>>
>> On 30/03/21 6:21 am, Steven Rostedt wrote:
>>> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>>>
>>> All for full "regex(3)" processing of setting functions in the
>>> set_ftrace_filter file. Check if the filter passed in is just a glob
>>> expression that the kernel can process, or if it is a regex that should look
>>> at the available_filter_functions list instead.
>>>
>>> If it is a regex, it will read the available_filter_functions and write in
>>> each function as it finds it.
>>>
>>> Link: https://lore.kernel.org/linux-trace-devel/20210323013225.451281989@goodmis.org
>>>
>>> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>> ---
>>> Documentation/libtracefs-function-filter.txt | 10 ++
>>> src/tracefs-tools.c | 139 ++++++++++++++++---
>>> 2 files changed, 128 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/Documentation/libtracefs-function-filter.txt b/Documentation/libtracefs-function-filter.txt
>>> index c0c89f372c21..88aa3b923d54 100644
>>> --- a/Documentation/libtracefs-function-filter.txt
>>> +++ b/Documentation/libtracefs-function-filter.txt
>>> @@ -32,6 +32,16 @@ _errs_, is a pointer to an array of strings, which will be allocated if
>>> any of filters fail to match any available function, If _errs_ is NULL, it will
>>> be ignored.
>>>
>>> +A filter in the array of _filters_ may be either a straight match of a
>>> +function, a glob or regex(3). a glob is where '*' matches zero or more
>>> +characters, '?' will match zero or one character, and '.' only matches a
>>> +period. If the filter is determined to be a regex (where it contains
>>> +anything other than alpha numeric characters, or '.', '*', '?') the filter
>>> +will be processed as a regex(3) following the rules of regex(3), and '.' is
>>> +not a period, but will match any one character. To force a regular
>>> +expression, either prefix the filter with a '^' or append it with a '$' as
>>> +all filters will act as complete matches of functions anyway.
>>> +
>> if we give the filter as regex "^ext4*$" from user side, ideally it
>> should match the ext4 filter functions, if i am not wrong, its not
>> matching any filter in the available_filter_functions
> If you add the "^" and/or "$" it makes it into a regex. The above means
> that the filter will match "ext", "ext4", "ext44", "ext4444444"
>
> Because "*" means zero or more of the previous character. So, unless
> there's a function that matches one of the above, it wont match anything
> else.
>
> If you left off the "^" and "$" then it would be a glob, where "*" means
> zero or more of any character. But if you want the same in regex, you need
> to use:
>
> "^ext4.*$"
IF we use the regex in filters, running time of the program is
increasing drastically.
lets say,
if we give the kernel glob as a filter, its getting converted to regex
and running time of program is
5 secs, in other case where we use regex, its taking 80 secs to complete.
--sameer.
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2021-04-01 2:42 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-30 0:51 [PATCH 00/13 v2] libtracefs: Add tracefs_function_filter() Steven Rostedt
2021-03-30 0:51 ` [PATCH 01/13 v2] libtracefs: An API to set the filtering of functions Steven Rostedt
2021-03-30 0:51 ` [PATCH 02/13 v2] libtracefs: Document function_filter API Steven Rostedt
2021-03-30 0:51 ` [PATCH 03/13 v2] libtracefs: Fix notations of tracefs_function_filter() and module parameter Steven Rostedt
2021-03-30 0:51 ` [PATCH 04/13 v2] libtracefs: Move opening of file out of controlled_write() Steven Rostedt
2021-03-30 0:51 ` [PATCH 05/13 v2] libtracefs: Add add_errors() helper function for control_write() Steven Rostedt
2021-03-30 0:51 ` [PATCH 06/13 v2] libtracefs: Add checking of available_filter_functions to tracefs_function_filter() Steven Rostedt
2021-03-30 0:51 ` [PATCH 07/13 v2] libtracefs: Add write_filter() helper function Steven Rostedt
2021-03-30 0:51 ` [PATCH 08/13 v2] libtracefs: Allow for setting filters with regex expressions Steven Rostedt
2021-04-01 16:33 ` sameeruddin shaik
2021-03-31 16:39 ` Steven Rostedt
2021-04-02 1:59 ` sameeruddin shaik
2021-04-01 2:41 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 09/13 v2] libtracefs: Add indexing to set functions in tracefs_function_filter() Steven Rostedt
2021-03-30 0:51 ` [PATCH 10/13 v2] libtracefs: Pass in reset via flags to tracefs_function_filter() Steven Rostedt
2021-03-30 14:29 ` Tzvetomir Stoyanov
2021-03-30 14:53 ` Steven Rostedt
2021-03-30 0:51 ` [PATCH 11/13 v2] libtracefs: Add pthread_mutex_lock() around tracefs_function_filter() Steven Rostedt
2021-03-30 0:51 ` [PATCH 12/13 v2] libtracefs: Move struct tracefs_instance to tracefs-local.h Steven Rostedt
2021-03-30 0:51 ` [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter() Steven Rostedt
2021-03-30 14:29 ` Tzvetomir Stoyanov
2021-03-30 14:52 ` Steven Rostedt
2021-03-30 15:14 ` Steven Rostedt
2021-03-30 15:32 ` Tzvetomir Stoyanov
2021-03-30 16:03 ` Steven Rostedt
2021-03-30 1:03 ` [RFC][PATCH 14/13 v2] libtracefs: Just past one filter in for tracefs_function_filter() Steven Rostedt
2021-03-30 14:31 ` 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).