Hi steve,
I have doubt
On Tue, Mar 23, 2021 at 6:22 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Mon, 22 Mar 2021 21:27:55 -0400
> Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> > This adds a new API tracefs_function_filter() as described in:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=210643
> >
> > It will use regular expressions against available_filter_functions (or even
> > the kernel glob expression) to enable the function by the index method if it
> > is supported. If it is not supported, it will go back to the writing of the
> > filter strings directly into the set_ftrace_filter file.
> >
> >
>
> Playing with the interface some more, I feel it's not quite adequate.
>
> The returning the negative number of filters that failed, isn't very
> useful. Having the errs array that points to those filters gives us that
> information.
>
> Also, because of the way that file works in the kernel, we need to be able
> to call this function several times without closing the file. That's
> because the actions take place when the file is closed, *not* when a write
> is made. Having the interface return errors without closing the file would
> allow the user to fix the failed filters and try again.
>
> I plan on committing this patch series, so any new changes will be done on
> top of them. But here's what I think needs to be done to make the interface
> more usable.
>
> - Create a tracefs_function_filter_commit(instance) API that will close
> the file and commit the changes.
>
> - Have the tracefs_function_filter() open the set_ftrace_filter file if it
> is not already opened. This will allow for the function to be called
> multiple times. The file descriptor will be part of the instance
> descriptor or a global variable for the top level instance. The opening
> of the files will need to be protected by a pthread mutex.
> Note, the "reset" parameter is only applicable if the file is not
> already opened.
>
> - On success, return 0 and tracefs_function_filter_commit() must be called.
>
> - If the file descriptor is opened and an error happens, return 1, and the
> tracefs_function_filter_commit() still needs to be called, but the user
> can call tracefs_function_filter() again to try to fix the problem.
>
> - If an error is found before the file descriptor is opened, then
> tracefs_function_filter_commit() does not need to be called.
>
> That is:
>
> ret = tracefs_function_filter(...);
> if (ret >= 0)
> tracefs_function_filter_commit();
>
> - Now for errs, it will be allocated if a problem happened on a filter,
> and may be set if the return value is non zero (1 or -1). The user can
> use it to know if the problem happened on a filter as well as find out
> which filter had the problem.
>
> - We can have a tracefs_read_function_filter() that returns an array of
> strings which ends with a NULL pointer (and needs to be freed with
> tracefs_list_free(), and have this:
>
> save_filters = tracefs_read_function_filter(instance);
are we going to use the tracefs_read_function_filter() function to
read the set_ftrace_file for getting written filters?
>
> ret = tracefs_function_filter(instance, filters, NULL, true, &errs);
> if (ret > 0 && errs) {
> /* Modified but failed */
> int i, j;
> for (i = 0; filters[i]; i++)
> ;
> for (j = 0; errs[i]; j++)
> ;
> if (i == j)
> /* all filters failed! Put back the original */
> tracefs_function_filter(instance, save_filters, NULL, false, NULL);
> }
> if (ret >= 0)
> tracefs_function_filter_commit(instance);
>
> Another reason to have it not close the file is because we only support
> passing in one module at a time. If the user wants to enable functions in
> two modules, they would need to call this twice, and we want them to be
> able to do so and have both changes take affect at the same time.
>
> Thoughts?
>
> -- Steve
Thanks,
sameer.
|