Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: sameeruddin shaik <sameeruddin.shaik8@gmail.com>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH 0/7] libtracfes: Add tracefs_function_filter()
Date: Wed, 24 Mar 2021 20:35:38 -0400
Message-ID: <20210324203538.7bbe7f77@oasis.local.home> (raw)
In-Reply-To: <743080c8-6b22-83b7-6b2b-0221c2ee945b@gmail.com>

On Fri, 26 Mar 2021 05:55:46 +0530
sameeruddin shaik <sameeruddin.shaik8@gmail.com> wrote:

> 
> On 23/03/21 6:22 pm, Steven Rostedt wrote:
> > On Mon, 22 Mar 2021 21:27:55 -0400
> > Steven Rostedt <rostedt@goodmis.org> 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.
> Yeah, Anyway we are pointing the failed filters using the errs pointer. 
> But to differentiate
> 
> the failed filters from the general errors, we can return -1, what you say?
>

No, I was thinking that -1 means that the file was never opened, but 1
means an error happened after the file was opened (and thus it must be
closed, or "committed"). In either case, the errs can be set.

I have the function checking the available_filter_functions to see if
there's any matches before it even opens the file, and if any filter
fails, I was thinking it should return -1 (meaning failure before
opening) and the errs will be set to the filters that failed (if it was
due to a filter failing. We should clear errno, at the start, so that
if a filter failed, errno is zero, and errs points to the failed
filters, if there was another failure, then errno would be set instead).

If all filters match, and we use your old method and there's some other
kind of error in writing the filter to set_ftrace_filter, then we could
have errs point to that filter, but still return 1 because the file was
opened and it still needs to be closed / committed. Note, this wont
happen on kernels 5.1 and beyond, as they support indexing, and once we
get to the indexing part, errs wont be used.

-- Steve

      reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-23  1:27 Steven Rostedt
2021-03-23  1:27 ` [PATCH 1/7] libtracefs: An API to set the filtering of functions Steven Rostedt
2021-03-23  1:27 ` [PATCH 2/7] libtracefs: Move opening of file out of controlled_write() Steven Rostedt
2021-03-23  1:27 ` [PATCH 3/7] libtracefs: Add add_errors() helper function for control_write() Steven Rostedt
2021-03-23  1:27 ` [PATCH 4/7] libtracefs: Add checking of available_filter_functions to tracefs_function_filter() Steven Rostedt
2021-03-23  1:28 ` [PATCH 5/7] libtracefs: Add write_filter() helper function Steven Rostedt
2021-03-23  1:28 ` [PATCH 6/7] libtracefs: Allow for setting filters with regex expressions Steven Rostedt
2021-03-23  1:28 ` [PATCH 7/7] libtracefs: Add indexing to set functions in tracefs_function_filter() Steven Rostedt
2021-03-23 12:52 ` [PATCH 0/7] libtracfes: Add tracefs_function_filter() Steven Rostedt
2021-03-24 10:13   ` Sameeruddin Shaik
2021-03-24 14:08     ` Steven Rostedt
2021-03-26  0:25   ` sameeruddin shaik
2021-03-25  0:35     ` Steven Rostedt [this message]

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=20210324203538.7bbe7f77@oasis.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-devel@vger.kernel.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