From: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Linux Trace Devel <linux-trace-devel@vger.kernel.org>,
Sameeruddin shaik <sameeruddin.shaik8@gmail.com>
Subject: Re: [PATCH 13/13 v2] libtracefs: Add CONTINUE to tracefs_function_filter()
Date: Tue, 30 Mar 2021 18:32:25 +0300 [thread overview]
Message-ID: <CAPpZLN4cC4Z7eYP=VTj09XRhQ_jaswcgYXa1MEnvs7RJRVBCeA@mail.gmail.com> (raw)
In-Reply-To: <20210330105250.0a7105a1@gandalf.local.home>
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
next prev parent reply other threads:[~2021-03-30 15:33 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPpZLN4cC4Z7eYP=VTj09XRhQ_jaswcgYXa1MEnvs7RJRVBCeA@mail.gmail.com' \
--to=tz.stoyanov@gmail.com \
--cc=linux-trace-devel@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=sameeruddin.shaik8@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).