| Message ID | 1614705310-5887-1-git-send-email-sameeruddin.shaik8@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libtracefs: An API to set the filtering of functions | expand |
On Tue, 2 Mar 2021 22:45:10 +0530 Sameeruddin shaik <sameeruddin.shaik8@gmail.com> wrote: > This new API will write the function filters into the > set_ftrace_filter file, it will write only string as of now, it can't > handle kernel glob or regular expressions. The limitation of no glob or regular expressions is fine. We can add that in future patches. > > tracefs_function_filter() > > https://bugzilla.kernel.org/show_bug.cgi?id=210643 > > Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com> > > diff --git a/include/tracefs.h b/include/tracefs.h > index f3eec62..b5259f9 100644 > --- a/include/tracefs.h > +++ b/include/tracefs.h > @@ -50,6 +50,7 @@ int tracefs_trace_on(struct tracefs_instance *instance); > int tracefs_trace_off(struct tracefs_instance *instance); > int tracefs_trace_on_fd(int fd); > int tracefs_trace_off_fd(int fd); > +int tracefs_function_filter(struct tracefs_instance *instance, const char * const *filters, const char *module, bool reset); > > /** > * tracefs_trace_on_get_fd - Get a file descriptor of "tracing_on" in given instance > diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c > index e2dfc7b..b8d8c99 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,85 @@ 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 * const *filters, const char *module, bool reset) > +{ > + int write_size; > + char *each_str; > + int size = 0; > + int slen; > + int fd; > + int i; > + > + Only need one empty line between declaration and code. > + if (!filters) > + return -1; > + if (reset) > + fd = open(filter_path, O_WRONLY | O_TRUNC); > + else > + fd = open(filter_path, O_WRONLY | O_APPEND); It's usually more robust to have a system call parameter set by an if statement and only call the system call once. int flags; flags = reset ? O_TRUNC : O_APPEND; Although O_APPEND isn't really needed here (zero would work), but it's fine to have. fd = open(filter_path, O_WRONLY | flags); > + if (fd < 0) > + return -1; I would add a blank line between the check and for loop, to space out the logic a bit better. fd = open(filter_path, O_WRONLY | flags); if (fd < 0) return -1; for (i = 0; filters[i]; i++) No space between setting the fd and checking it. It gives a visual on how code is related. > + for (i = 0; filters[i] != NULL ; i++) { No need for the " != NULL" > + slen = 0; > + slen = strlen(*(filters + i)); No need for slen = 0 as you set it immediately afterward, also, it is more appropriate to use: slen = strlen(filters[i]); > + if (slen < 0) The above can't happen, strlen always returns a positive integer (or zero). Now checking for zero would make sense. if (!slen) > + continue; > + > + if (module) > + slen += strlen(module) + 5; What's the "+ 5" for? Should be commented. > + /* Adding 2 extra byte for the space and '\0' at the end*/ > + slen += 2; > + each_str = calloc(1, slen); > + if (!each_str) > + return -1; > + if (module) > + write_size = snprintf(each_str, slen, "%s:mod:%s ", *(filters + i), module); > + else > + write_size = snprintf(each_str, slen, "%s ", *(filters + i)); > + if (write_size < (slen - 1)) { This should never happen. If it does, then there's something wrong with this code, not the input. > + free(each_str); > + continue; > + } > + size += write(fd, each_str, write_size); Need to check errors here, and we need a way to report that an error happened for a string. Perhaps the API also needs to have an error message pointer that is passed in? Or a bitmask that lets the user know which filter failed? Tzvetomir, have any ideas on how to report back to the caller which filter failed? Could just send back an array of strings of the filters that failed. int tracefs_function_filter(struct tracefs_instance *instance, const char * const * filters, const char * module, bool reset, const char * const ** errs); Where if errs points to a pointer, that pointer gets updated with an array. const char **filters = { "func1", "func2", func3" }; const char **errs; ret; ret = tracefs_function_filter(NULL, filters, NULL, &errs); If "func1" and "func3" fail, then ret is < 0 (-2 for number of failures?) and errs would be: errs = { &filter[0], &filters[2], NULL }; and the users would need to free it as well. free(errs); if ret is zero, then errs would not be touched, and the caller does not need to do anything with it. Note, if the user doesn't care about errors, errs could be NULL, in which case the calling function would obviously not set it. > + free(each_str); > + } > + close(fd); > + return size; > +} > + > +/** > + * tracefs_function_filter - write to set_ftrace_filter file to trace particular functions > + * @instance: ftrace instance, can be NULL for top tracing instance > + * @filter: An array of function names ending with a NULL pointer > + * @module: Module Name to be traced > + * @reset: set to true to reset the file before applying the filter > + * > + * The @filter is an array of strings, where each string will be use 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 @filter. Otherwise, the functions set by @filter > + * will be appended to the filter > + * > + * Returns the number of bytes written into the filter file or -1 if > + * there is any error in writing to filter file > + */ > +int tracefs_function_filter(struct tracefs_instance *instance, const char * const *filters, const char *module, bool reset) > +{ > + char *ftrace_filter_path; > + int ret; > + > + if (!filters) > + return -1; You can add a empty line here. > + ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER); > + > + if (!ftrace_filter_path) > + goto gracefully_free; It should just be "goto out". > + > + ret = controlled_write(ftrace_filter_path, filters, module, reset); > + > + gracefully_free: > + tracefs_put_tracing_file(ftrace_filter_path); > + return ret; Note, you need to initialize ret to -1, otherwise it is undefined if you do the "goto out". -- Steve > +}
On Wed, 3 Mar 2021 06:46:26 +0530 Sameeruddin Shaik <sameeruddin.shaik8@gmail.com> wrote: > what if we store the indices of the failed filters in an integer array > and return them back? There's not much difference if we return an array of pointers to the filters or an array of integers to the index. I was just thinking about how I would use the interface. When having a working interface, we should write a few robust programs to see how easy it is to use, and that will help in making the API appropriate. This needs to be done *before* we accept it. This particular API is going to be widely used, and it needs to be simple and robust. > let's return the number of bytes written, also we will calculate the > complete filters length and return it, if there is difference, > we will loop into the integer array and print the erroneous filters Not sure how that is helpful. How would you use the number of bytes written? > > Let's fix the number of parameters to this function:) Not sure what you mean by that. Here's how I envision this interface. char **errs; char *filters[] = { "sched*", "spin_*", NULL }; int ret; ret = tracefs_function_filter(NULL, filters, NULL, &errs); if (ret < 0) { int i; printf("Failed to apply: "); for (i = 0; errs[i]; i++) { if (i) printf(", "); printf("'%s'", errs[i]); } printf("\n"; exit(ret); } -- Steve
On Mon, Mar 1, 2021 at 10:40 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, 2 Mar 2021 22:45:10 +0530 > Sameeruddin shaik <sameeruddin.shaik8@gmail.com> wrote: > > > This new API will write the function filters into the > > set_ftrace_filter file, it will write only string as of now, it can't > > handle kernel glob or regular expressions. > > The limitation of no glob or regular expressions is fine. We can add that > in future patches. > > > > > tracefs_function_filter() > > > > https://bugzilla.kernel.org/show_bug.cgi?id=210643 > > > > Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com> > > [ ... ] > > > + free(each_str); > > + continue; > > + } > > + size += write(fd, each_str, write_size); > > Need to check errors here, and we need a way to report that an error > happened for a string. Perhaps the API also needs to have an error message > pointer that is passed in? Or a bitmask that lets the user know which > filter failed? > > Tzvetomir, have any ideas on how to report back to the caller which filter > failed? Could just send back an array of strings of the filters that failed. It is very useful to have a way to report back the failed filters. Using an array of strings will work for this API, but I was thinking somehow to leverage the error_log file by the ftrace itself. Currently it does not report any error, just returns EINVAL. In more complex filters it would be useful to log more detailed description of the problem in the error_log file. [ ... ]
> Let's fix the number of parameters to this function:) >> Not sure what you mean by that. Actually i meant this ""int tracefs_function_filter(struct tracefs_instance *instance, const char * const * filters, const char * module, bool reset, const char * const ** errs);"" For every patch, a parameter is increasing in this API. > let's return the number of bytes written, also we will calculate the > complete filters length and return it, if there is difference, > we will loop into the integer array and print the erroneous filters >>Not sure how that is helpful. How would you use the number of bytes >>written? We will return the number of bytes written and we also store the total length of strings in filters array, in one pointer variable, we will check the difference between bytes written and the total length of the strings, if there is difference we will print failed filters otherwise we will not print anything. >It is very useful to have a way to report back the failed filters. >Using an array >of strings will work for this API, but I was thinking somehow to leverage the >error_log file by the ftrace itself. Currently it does not report any >error, just >returns EINVAL. In more complex filters it would be useful to log >more detailed description of the problem in the error_log file. This error_log is also a good idea. If possible let's have a live discussion on this API,so that we can discuss the corner cases in the design more efficiently and we can close it ASAP. Thanks, sameer shaik. Thanks, sameer shaik On Tue, Mar 2, 2021 at 9:52 AM Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote: > > On Mon, Mar 1, 2021 at 10:40 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Tue, 2 Mar 2021 22:45:10 +0530 > > Sameeruddin shaik <sameeruddin.shaik8@gmail.com> wrote: > > > > > This new API will write the function filters into the > > > set_ftrace_filter file, it will write only string as of now, it can't > > > handle kernel glob or regular expressions. > > > > The limitation of no glob or regular expressions is fine. We can add that > > in future patches. > > > > > > > > tracefs_function_filter() > > > > > > https://bugzilla.kernel.org/show_bug.cgi?id=210643 > > > > > > Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com> > > > > [ ... ] > > > > > + free(each_str); > > > + continue; > > > + } > > > + size += write(fd, each_str, write_size); > > > > Need to check errors here, and we need a way to report that an error > > happened for a string. Perhaps the API also needs to have an error message > > pointer that is passed in? Or a bitmask that lets the user know which > > filter failed? > > > > Tzvetomir, have any ideas on how to report back to the caller which filter > > failed? Could just send back an array of strings of the filters that failed. > > It is very useful to have a way to report back the failed filters. > Using an array > of strings will work for this API, but I was thinking somehow to leverage the > error_log file by the ftrace itself. Currently it does not report any > error, just > returns EINVAL. In more complex filters it would be useful to log > more detailed description of the problem in the error_log file. > > [ ... ] > > > -- > Tzvetomir (Ceco) Stoyanov > VMware Open Source Technology Center
On Tue, 2 Mar 2021 10:44:14 +0530 Sameeruddin Shaik <sameeruddin.shaik8@gmail.com> wrote: > > Let's fix the number of parameters to this function:) > > >> Not sure what you mean by that. > Actually i meant this ""int tracefs_function_filter(struct > tracefs_instance *instance, > const char * const * filters, > const char * module, bool reset, > const char * const ** errs);"" > For every patch, a parameter is increasing in this API. And that's a normal approach to developing APIs. I've been doing this for over 25 years, there's nothing special about this case. In discussion on APIs, parameters grow to handle new cases. That's par for the course. There's only 5 parameters, not too many. > > > let's return the number of bytes written, also we will calculate the > > complete filters length and return it, if there is difference, > > we will loop into the integer array and print the erroneous filters > > >>Not sure how that is helpful. How would you use the number of bytes > >>written? > > We will return the number of bytes written and we also store the total > length of strings > in filters array, in one pointer variable, we will check the > difference between bytes written > and the total length of the strings, if there is difference we will > print failed filters otherwise > we will not print anything. Please show an example of a use case that you would use this with? I gave you an example of how I intend on using it, and the user of this interface should not care about number of bytes written. And the interface should not be printing any error messages, it is a library, error messages are for applications to produce. The interface must give the application enough information to be able to produce it. > > >It is very useful to have a way to report back the failed filters. > >Using an array > >of strings will work for this API, but I was thinking somehow to leverage the > >error_log file by the ftrace itself. Currently it does not report any > >error, just > >returns EINVAL. In more complex filters it would be useful to log > >more detailed description of the problem in the error_log file. > > This error_log is also a good idea. It's actually not very useful for this interface. The only error that it would give you is that it could not find any functions that match a filter. When we create other interfaces that do more than just setting functions (like setting triggers) then we can return to looking at the error log. But since the kernel does not produce anything in the error log at the moment, it must be updated. And kernels take about a year or two after a change to get into distributions. That means, this library can't rely on it being there, and still needs a way to inform the application of errors. > > If possible let's have a live discussion on this API,so that we can > discuss the corner cases in the design > more efficiently and we can close it ASAP. > I have a very good idea of what I want from this interface, which is why I created the bugzilla about it. If you want to implement it different than my idea, please show code examples of your use cases. -- Steve
what if we store the indices of the failed filters in an integer array and return them back? let's return the number of bytes written, also we will calculate the complete filters length and return it, if there is difference, we will loop into the integer array and print the erroneous filters Let's fix the number of parameters to this function:) Thanks, sameer. On Mon, Mar 1, 2021 at 11:47 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, 2 Mar 2021 22:45:10 +0530 > Sameeruddin shaik <sameeruddin.shaik8@gmail.com> wrote: > > > This new API will write the function filters into the > > set_ftrace_filter file, it will write only string as of now, it can't > > handle kernel glob or regular expressions. > > The limitation of no glob or regular expressions is fine. We can add that > in future patches. > > > > > tracefs_function_filter() > > > > https://bugzilla.kernel.org/show_bug.cgi?id=210643 > > > > Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com> > > > > diff --git a/include/tracefs.h b/include/tracefs.h > > index f3eec62..b5259f9 100644 > > --- a/include/tracefs.h > > +++ b/include/tracefs.h > > @@ -50,6 +50,7 @@ int tracefs_trace_on(struct tracefs_instance *instance); > > int tracefs_trace_off(struct tracefs_instance *instance); > > int tracefs_trace_on_fd(int fd); > > int tracefs_trace_off_fd(int fd); > > +int tracefs_function_filter(struct tracefs_instance *instance, const char * const *filters, const char *module, bool reset); > > > > /** > > * tracefs_trace_on_get_fd - Get a file descriptor of "tracing_on" in given instance > > diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c > > index e2dfc7b..b8d8c99 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,85 @@ 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 * const *filters, const char *module, bool reset) > > +{ > > + int write_size; > > + char *each_str; > > + int size = 0; > > + int slen; > > + int fd; > > + int i; > > + > > + > > Only need one empty line between declaration and code. > > > + if (!filters) > > + return -1; > > + if (reset) > > + fd = open(filter_path, O_WRONLY | O_TRUNC); > > + else > > + fd = open(filter_path, O_WRONLY | O_APPEND); > > It's usually more robust to have a system call parameter set by an if > statement and only call the system call once. > > int flags; > > flags = reset ? O_TRUNC : O_APPEND; > > Although O_APPEND isn't really needed here (zero would work), but it's fine > to have. > > fd = open(filter_path, O_WRONLY | flags); > > > > + if (fd < 0) > > + return -1; > > I would add a blank line between the check and for loop, to space out the > logic a bit better. > > fd = open(filter_path, O_WRONLY | flags); > if (fd < 0) > return -1; > > for (i = 0; filters[i]; i++) > > No space between setting the fd and checking it. It gives a visual on how > code is related. > > > + for (i = 0; filters[i] != NULL ; i++) { > > No need for the " != NULL" > > > > + slen = 0; > > + slen = strlen(*(filters + i)); > > No need for slen = 0 as you set it immediately afterward, also, it is more > appropriate to use: > > slen = strlen(filters[i]); > > > > + if (slen < 0) > > The above can't happen, strlen always returns a positive integer (or zero). > Now checking for zero would make sense. > > if (!slen) > > + continue; > > > > + > > + if (module) > > + slen += strlen(module) + 5; > > What's the "+ 5" for? Should be commented. > > > + /* Adding 2 extra byte for the space and '\0' at the end*/ > > + slen += 2; > > + each_str = calloc(1, slen); > > + if (!each_str) > > + return -1; > > + if (module) > > + write_size = snprintf(each_str, slen, "%s:mod:%s ", *(filters + i), module); > > + else > > + write_size = snprintf(each_str, slen, "%s ", *(filters + i)); > > + if (write_size < (slen - 1)) { > > This should never happen. If it does, then there's something wrong with > this code, not the input. > > > > + free(each_str); > > + continue; > > + } > > + size += write(fd, each_str, write_size); > > Need to check errors here, and we need a way to report that an error > happened for a string. Perhaps the API also needs to have an error message > pointer that is passed in? Or a bitmask that lets the user know which > filter failed? > > Tzvetomir, have any ideas on how to report back to the caller which filter > failed? Could just send back an array of strings of the filters that failed. > > int tracefs_function_filter(struct tracefs_instance *instance, > const char * const * filters, > const char * module, bool reset, > const char * const ** errs); > > Where if errs points to a pointer, that pointer gets updated with an array. > > const char **filters = { "func1", "func2", func3" }; > const char **errs; > ret; > > ret = tracefs_function_filter(NULL, filters, NULL, &errs); > > If "func1" and "func3" fail, then ret is < 0 (-2 for number of failures?) > and errs would be: > > errs = { &filter[0], &filters[2], NULL }; > > and the users would need to free it as well. > > free(errs); > > if ret is zero, then errs would not be touched, and the caller does not > need to do anything with it. > > Note, if the user doesn't care about errors, errs could be NULL, in which > case the calling function would obviously not set it. > > > > > + free(each_str); > > + } > > + close(fd); > > + return size; > > +} > > + > > +/** > > + * tracefs_function_filter - write to set_ftrace_filter file to trace particular functions > > + * @instance: ftrace instance, can be NULL for top tracing instance > > + * @filter: An array of function names ending with a NULL pointer > > + * @module: Module Name to be traced > > + * @reset: set to true to reset the file before applying the filter > > + * > > + * The @filter is an array of strings, where each string will be use 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 @filter. Otherwise, the functions set by @filter > > + * will be appended to the filter > > + * > > + * Returns the number of bytes written into the filter file or -1 if > > + * there is any error in writing to filter file > > + */ > > +int tracefs_function_filter(struct tracefs_instance *instance, const char * const *filters, const char *module, bool reset) > > +{ > > + char *ftrace_filter_path; > > + int ret; > > + > > + if (!filters) > > + return -1; > > You can add a empty line here. > > > + ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER); > > + > > + if (!ftrace_filter_path) > > + goto gracefully_free; > > It should just be "goto out". > > > + > > + ret = controlled_write(ftrace_filter_path, filters, module, reset); > > + > > + gracefully_free: > > + tracefs_put_tracing_file(ftrace_filter_path); > > + return ret; > > Note, you need to initialize ret to -1, otherwise it is undefined if you > do the "goto out". > > -- Steve > > > +} >
On Thu, Mar 4, 2021 at 9:24 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Wed, 3 Mar 2021 06:46:26 +0530 > Sameeruddin Shaik <sameeruddin.shaik8@gmail.com> wrote: > > > what if we store the indices of the failed filters in an integer array > > and return them back? > > There's not much difference if we return an array of pointers to the > filters or an array of integers to the index. I was just thinking about > how I would use the interface. When having a working interface, we > should write a few robust programs to see how easy it is to use, and > that will help in making the API appropriate. This needs to be done > *before* we accept it. This particular API is going to be widely used, > and it needs to be simple and robust. One remark, not directly related to this discussion. When the implementation of the API is ready, there should be a unit test (in a separate patch) - as for any of the other APIs. Usually these are the first use cases that I write for the new APIs. Sameer, please look at utest directory where the unit tests are, each API has a unit test there. We use the cunit framework, ask if there are questions about it. This should be the next step, when the final version of the implementation is ready. Thanks! > > > let's return the number of bytes written, also we will calculate the > > complete filters length and return it, if there is difference, > > we will loop into the integer array and print the erroneous filters > > Not sure how that is helpful. How would you use the number of bytes > written? > > > > > Let's fix the number of parameters to this function:) > > Not sure what you mean by that. > > > Here's how I envision this interface. > > char **errs; > char *filters[] = { > "sched*", "spin_*", NULL > }; > int ret; > > > ret = tracefs_function_filter(NULL, filters, NULL, &errs); > if (ret < 0) { > int i; > > printf("Failed to apply: "); > for (i = 0; errs[i]; i++) { > if (i) > printf(", "); > printf("'%s'", errs[i]); > } > printf("\n"; > exit(ret); > } > > -- Steve
sure tzvetomir. On Thu, Mar 4, 2021 at 2:29 PM Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote: > > On Thu, Mar 4, 2021 at 9:24 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Wed, 3 Mar 2021 06:46:26 +0530 > > Sameeruddin Shaik <sameeruddin.shaik8@gmail.com> wrote: > > > > > what if we store the indices of the failed filters in an integer array > > > and return them back? > > > > There's not much difference if we return an array of pointers to the > > filters or an array of integers to the index. I was just thinking about > > how I would use the interface. When having a working interface, we > > should write a few robust programs to see how easy it is to use, and > > that will help in making the API appropriate. This needs to be done > > *before* we accept it. This particular API is going to be widely used, > > and it needs to be simple and robust. > > One remark, not directly related to this discussion. When the implementation > of the API is ready, there should be a unit test (in a separate patch) - as for > any of the other APIs. Usually these are the first use cases that I write for > the new APIs. > Sameer, please look at utest directory where the unit tests are, each API has > a unit test there. We use the cunit framework, ask if there are questions about > it. This should be the next step, when the final version of the > implementation is > ready. > Thanks! > > > > > > let's return the number of bytes written, also we will calculate the > > > complete filters length and return it, if there is difference, > > > we will loop into the integer array and print the erroneous filters > > > > Not sure how that is helpful. How would you use the number of bytes > > written? > > > > > > > > Let's fix the number of parameters to this function:) > > > > Not sure what you mean by that. > > > > > > Here's how I envision this interface. > > > > char **errs; > > char *filters[] = { > > "sched*", "spin_*", NULL > > }; > > int ret; > > > > > > ret = tracefs_function_filter(NULL, filters, NULL, &errs); > > if (ret < 0) { > > int i; > > > > printf("Failed to apply: "); > > for (i = 0; errs[i]; i++) { > > if (i) > > printf(", "); > > printf("'%s'", errs[i]); > > } > > printf("\n"; > > exit(ret); > > } > > > > -- Steve > > > > -- > Tzvetomir (Ceco) Stoyanov > VMware Open Source Technology Center
diff --git a/include/tracefs.h b/include/tracefs.h index f3eec62..b5259f9 100644 --- a/include/tracefs.h +++ b/include/tracefs.h @@ -50,6 +50,7 @@ int tracefs_trace_on(struct tracefs_instance *instance); int tracefs_trace_off(struct tracefs_instance *instance); int tracefs_trace_on_fd(int fd); int tracefs_trace_off_fd(int fd); +int tracefs_function_filter(struct tracefs_instance *instance, const char * const *filters, const char *module, bool reset); /** * tracefs_trace_on_get_fd - Get a file descriptor of "tracing_on" in given instance diff --git a/src/tracefs-tools.c b/src/tracefs-tools.c index e2dfc7b..b8d8c99 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,85 @@ 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 * const *filters, const char *module, bool reset) +{ + int write_size; + char *each_str; + int size = 0; + int slen; + int fd; + int i; + + + if (!filters) + return -1; + if (reset) + fd = open(filter_path, O_WRONLY | O_TRUNC); + else + fd = open(filter_path, O_WRONLY | O_APPEND); + if (fd < 0) + return -1; + for (i = 0; filters[i] != NULL ; i++) { + slen = 0; + slen = strlen(*(filters + i)); + if (slen < 0) + continue; + + if (module) + slen += strlen(module) + 5; + /* Adding 2 extra byte for the space and '\0' at the end*/ + slen += 2; + each_str = calloc(1, slen); + if (!each_str) + return -1; + if (module) + write_size = snprintf(each_str, slen, "%s:mod:%s ", *(filters + i), module); + else + write_size = snprintf(each_str, slen, "%s ", *(filters + i)); + if (write_size < (slen - 1)) { + free(each_str); + continue; + } + size += write(fd, each_str, write_size); + free(each_str); + } + close(fd); + return size; +} + +/** + * tracefs_function_filter - write to set_ftrace_filter file to trace particular functions + * @instance: ftrace instance, can be NULL for top tracing instance + * @filter: An array of function names ending with a NULL pointer + * @module: Module Name to be traced + * @reset: set to true to reset the file before applying the filter + * + * The @filter is an array of strings, where each string will be use 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 @filter. Otherwise, the functions set by @filter + * will be appended to the filter + * + * Returns the number of bytes written into the filter file or -1 if + * there is any error in writing to filter file + */ +int tracefs_function_filter(struct tracefs_instance *instance, const char * const *filters, const char *module, bool reset) +{ + char *ftrace_filter_path; + int ret; + + if (!filters) + return -1; + ftrace_filter_path = tracefs_instance_get_file(instance, TRACE_FILTER); + + if (!ftrace_filter_path) + goto gracefully_free; + + ret = controlled_write(ftrace_filter_path, filters, module, reset); + + gracefully_free: + tracefs_put_tracing_file(ftrace_filter_path); + return ret; +}
This new API will write the function filters into the set_ftrace_filter file, it will write only string as of now, it can't handle kernel glob or regular expressions. tracefs_function_filter() https://bugzilla.kernel.org/show_bug.cgi?id=210643 Signed-off-by: Sameeruddin shaik <sameeruddin.shaik8@gmail.com>