On Tue, 30 Mar 2021 18:32:25 +0300
Tzvetomir Stoyanov <tz.stoyanov@xxxxxxxxx> wrote:
> On Tue, Mar 30, 2021 at 5:52 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> >
> > On Tue, 30 Mar 2021 17:29:57 +0300
> > Tzvetomir Stoyanov <tz.stoyanov@xxxxxxxxx> wrote:
> >
> > > On Tue, Mar 30, 2021 at 3:54 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> > > >
> > > > From: "Steven Rostedt (VMware)" <rostedt@xxxxxxxxxxx>
> > > >
> > > > 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@xxxxxxxxxxx>
> > > > ---
>
>
>
|