[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1249618497.30024.36.camel@tropicana>
Date: Thu, 06 Aug 2009 23:14:57 -0500
From: Tom Zanussi <tzanussi@...il.com>
To: Frederic Weisbecker <fweisbec@...il.com>
Cc: Li Zefan <lizf@...fujitsu.com>, Ingo Molnar <mingo@...e.hu>,
LKML <linux-kernel@...r.kernel.org>,
Steven Rostedt <rostedt@...dmis.org>,
Lai Jiangshan <laijs@...fujitsu.com>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [RFC][PATCH 4/5] tracing/filters: Provide basic regex support
Hi Frederic,
On Thu, 2009-08-06 at 03:49 +0200, Frederic Weisbecker wrote:
> On Thu, Aug 06, 2009 at 09:14:27AM +0800, Li Zefan wrote:
> > Frederic Weisbecker wrote:
> > > On Mon, Aug 03, 2009 at 01:39:58PM +0800, Li Zefan wrote:
> > >> Frederic Weisbecker wrote:
> > >>> This patch provides basic support for regular expressions in filters.
> > >>> The common filter file doesn't support any regex but a new
> > >>> filter_regex file is created for each subsystem/event.
> > >>>
> > >>> It supports the following types of regexp:
> > >>>
> > >>> - *match_beginning
> > >>> - *match_middle*
> > >>> - match_end*
> > >>> - !don't match
> > >>>
> > >> I don't see why adding "filter_regex" is necessary, why not just make
> > >> "filter" support regex?
> > >
> > > I did that because I feared about people beeing unable to filter using
> > > * as a real character inside a filter string.
> > > If we are using only one file, we are forced to get the '*' interpreted.
> > > That would also break the ABI.
> > >
> >
> > I think we don't maintain stable ABI for debugfs and we've been changing
> > the "ABI" in debugfs/tracing/, but of course we shouldn't change it
> > without good reasons.
>
>
> Yeah. May be I'm too much careful there but i prefer not to take the risk.
>
>
> > One alternative is to use '"' to prevent '*' to be translated, but seems
> > using "filter_regex" is more convenient, so I agree with this patch.
>
>
> The problem is that most of the time, the '"' is about mandatory to delimit
> a string.
> Say you want to filter the lock name &REISERFS_SB(sb)->lock, you can't do that
> by just typing:
>
> echo name == &REISERFS_SB(sb)->lock > events/lockdep/filter
>
> because the '&' character is considered as an operator and that will trigger
> an error.
> Instead you must type:
>
> echo 'name == "&REISERFS_SB(sb)->lock"' > events/lockdep/filter
>
> The '' are there to delimit the string for echo, and the "" are interpreted
> by the filter api which take it as a whole string and not strings interleaved
> with operators. I even fear - and > may be considered as operators if we omit
> the "".
>
> I also thought about using an antislash for those who don't want the * to
> be interpreted. But although it seems intuitive, it's not as much as two
> distinct and visible files.
>
> Also, we can think about the fact the regexp support could be extended one
> day if someone needs to. And we could then encounter even more ambiguous
> characters such as $, ^, [, {, etc...
>
This a nice new feature - I considered doing it (not complete regexp
support, just * in strings) for the original patch, but ran out of time
- glad you added it.
I still think it makes sense to have some basic support for * in the
regular filter file, so I'd vote for getting rid of the filter_regex
file for now and just adding * support with the antislash escape to the
regular filter file. If you later wanted to add more full-fledged
regexp support and it didn't make sense to do it in the regular filter
file, then you could go crazy and add the filter_regex later...
Thanks,
Tom
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists