Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
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 17:29:57 +0300
Message-ID: <CAPpZLN57JrvSNTCGJTe7A9_x9Z4rSsH54jQu5C8wqodii=np4w@mail.gmail.com> (raw)
In-Reply-To: <20210330005249.118764252@goodmis.org>

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

  reply index

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 [this message]
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

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='CAPpZLN57JrvSNTCGJTe7A9_x9Z4rSsH54jQu5C8wqodii=np4w@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

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org
	public-inbox-index linux-trace-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git