* [PATCH] perf tools: Add 'G' and 'H' modifiers to event parsing
@ 2012-04-15 14:59 Gleb Natapov
2012-04-15 15:12 ` Peter Zijlstra
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2012-04-15 14:59 UTC (permalink / raw)
To: acme; +Cc: a.p.zijlstra, mingo, jolsa, linux-kernel, avi
They were dropped during conversion of event parser.
Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 05d766e..37a3f17 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -54,7 +54,7 @@ num_dec [0-9]+
num_hex 0x[a-fA-F0-9]+
num_raw_hex [a-fA-F0-9]+
name [a-zA-Z_*?][a-zA-Z0-9_*?]*
-modifier_event [ukhp]{1,5}
+modifier_event [ukhpGH]{1,5}
modifier_bp [rwx]
%%
--
Gleb.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tools: Add 'G' and 'H' modifiers to event parsing
2012-04-15 14:59 [PATCH] perf tools: Add 'G' and 'H' modifiers to event parsing Gleb Natapov
@ 2012-04-15 15:12 ` Peter Zijlstra
2012-04-15 15:23 ` Gleb Natapov
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2012-04-15 15:12 UTC (permalink / raw)
To: Gleb Natapov; +Cc: acme, mingo, jolsa, linux-kernel, avi
On Sun, 2012-04-15 at 17:59 +0300, Gleb Natapov wrote:
> -modifier_event [ukhp]{1,5}
> +modifier_event [ukhpGH]{1,5}
You'd also need to increase the 5 to 7 I think, unless there's a clear
exclusion with some of the other flags.
Hmm, as it stands I think the 5 is already too low: "ukhppp" is 6.
We should really write this as ['u'] ['k'] ['h'] ['G'] ['H'] ['p'{1,3}],
not the combined thing.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tools: Add 'G' and 'H' modifiers to event parsing
2012-04-15 15:12 ` Peter Zijlstra
@ 2012-04-15 15:23 ` Gleb Natapov
2012-04-16 20:10 ` Jiri Olsa
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2012-04-15 15:23 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: acme, mingo, jolsa, linux-kernel, avi
On Sun, Apr 15, 2012 at 05:12:22PM +0200, Peter Zijlstra wrote:
> On Sun, 2012-04-15 at 17:59 +0300, Gleb Natapov wrote:
> > -modifier_event [ukhp]{1,5}
> > +modifier_event [ukhpGH]{1,5}
>
> You'd also need to increase the 5 to 7 I think, unless there's a clear
> exclusion with some of the other flags.
>
> Hmm, as it stands I think the 5 is already too low: "ukhppp" is 6.
>
> We should really write this as ['u'] ['k'] ['h'] ['G'] ['H'] ['p'{1,3}],
> not the combined thing.
If we will write them like that they all will have to be present for
successful match of modifier_event, no?
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tools: Add 'G' and 'H' modifiers to event parsing
2012-04-15 15:23 ` Gleb Natapov
@ 2012-04-16 20:10 ` Jiri Olsa
2012-04-17 9:37 ` Gleb Natapov
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2012-04-16 20:10 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Peter Zijlstra, acme, mingo, linux-kernel, avi
On Sun, Apr 15, 2012 at 06:23:00PM +0300, Gleb Natapov wrote:
> On Sun, Apr 15, 2012 at 05:12:22PM +0200, Peter Zijlstra wrote:
> > On Sun, 2012-04-15 at 17:59 +0300, Gleb Natapov wrote:
> > > -modifier_event [ukhp]{1,5}
> > > +modifier_event [ukhpGH]{1,5}
> >
> > You'd also need to increase the 5 to 7 I think, unless there's a clear
> > exclusion with some of the other flags.
> >
> > Hmm, as it stands I think the 5 is already too low: "ukhppp" is 6.
> >
> > We should really write this as ['u'] ['k'] ['h'] ['G'] ['H'] ['p'{1,3}],
> > not the combined thing.
you mean to match them strictly in parser, right?
because what we have now can match also stuff like 'uuu' 'kkuu' ...
So far it eludes me how to do that purely in flex and
doing this via rules in bison seems like overkill.
I'd rather keep it the way we have and do the extra check
in parse_events_modifier function.. unless someone comes up
with flex regular expression that would do that (which is
possible given my regex knowledge..)
also we need 8 characters for it, since this seems to be
valid modifier: ukhGHppp
jirka
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tools: Add 'G' and 'H' modifiers to event parsing
2012-04-16 20:10 ` Jiri Olsa
@ 2012-04-17 9:37 ` Gleb Natapov
2012-04-17 9:44 ` Jiri Olsa
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2012-04-17 9:37 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Peter Zijlstra, acme, mingo, linux-kernel, avi
On Mon, Apr 16, 2012 at 10:10:30PM +0200, Jiri Olsa wrote:
> On Sun, Apr 15, 2012 at 06:23:00PM +0300, Gleb Natapov wrote:
> > On Sun, Apr 15, 2012 at 05:12:22PM +0200, Peter Zijlstra wrote:
> > > On Sun, 2012-04-15 at 17:59 +0300, Gleb Natapov wrote:
> > > > -modifier_event [ukhp]{1,5}
> > > > +modifier_event [ukhpGH]{1,5}
> > >
> > > You'd also need to increase the 5 to 7 I think, unless there's a clear
> > > exclusion with some of the other flags.
> > >
> > > Hmm, as it stands I think the 5 is already too low: "ukhppp" is 6.
> > >
> > > We should really write this as ['u'] ['k'] ['h'] ['G'] ['H'] ['p'{1,3}],
> > > not the combined thing.
>
> you mean to match them strictly in parser, right?
> because what we have now can match also stuff like 'uuu' 'kkuu' ...
>
> So far it eludes me how to do that purely in flex and
> doing this via rules in bison seems like overkill.
>
> I'd rather keep it the way we have and do the extra check
> in parse_events_modifier function.. unless someone comes up
> with flex regular expression that would do that (which is
> possible given my regex knowledge..)
>
> also we need 8 characters for it, since this seems to be
> valid modifier: ukhGHppp
>
So want me to resend with 8 to fix missing GH for now? I can't think of
proper regex either.
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf tools: Add 'G' and 'H' modifiers to event parsing
2012-04-17 9:37 ` Gleb Natapov
@ 2012-04-17 9:44 ` Jiri Olsa
0 siblings, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2012-04-17 9:44 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Peter Zijlstra, acme, mingo, linux-kernel, avi
On Tue, Apr 17, 2012 at 12:37:13PM +0300, Gleb Natapov wrote:
> On Mon, Apr 16, 2012 at 10:10:30PM +0200, Jiri Olsa wrote:
> > On Sun, Apr 15, 2012 at 06:23:00PM +0300, Gleb Natapov wrote:
> > > On Sun, Apr 15, 2012 at 05:12:22PM +0200, Peter Zijlstra wrote:
> > > > On Sun, 2012-04-15 at 17:59 +0300, Gleb Natapov wrote:
> > > > > -modifier_event [ukhp]{1,5}
> > > > > +modifier_event [ukhpGH]{1,5}
> > > >
> > > > You'd also need to increase the 5 to 7 I think, unless there's a clear
> > > > exclusion with some of the other flags.
> > > >
> > > > Hmm, as it stands I think the 5 is already too low: "ukhppp" is 6.
> > > >
> > > > We should really write this as ['u'] ['k'] ['h'] ['G'] ['H'] ['p'{1,3}],
> > > > not the combined thing.
> >
> > you mean to match them strictly in parser, right?
> > because what we have now can match also stuff like 'uuu' 'kkuu' ...
> >
> > So far it eludes me how to do that purely in flex and
> > doing this via rules in bison seems like overkill.
> >
> > I'd rather keep it the way we have and do the extra check
> > in parse_events_modifier function.. unless someone comes up
> > with flex regular expression that would do that (which is
> > possible given my regex knowledge..)
> >
> > also we need 8 characters for it, since this seems to be
> > valid modifier: ukhGHppp
> >
> So want me to resend with 8 to fix missing GH for now? I can't think of
> proper regex either.
That would be great.. any chance you could add a test to the builtin-test.c? ;)
Or just modify current one (one of the test__parse_events tests) to use those
flags, should be straightforward..
thanks,
jirka
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-17 9:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-15 14:59 [PATCH] perf tools: Add 'G' and 'H' modifiers to event parsing Gleb Natapov
2012-04-15 15:12 ` Peter Zijlstra
2012-04-15 15:23 ` Gleb Natapov
2012-04-16 20:10 ` Jiri Olsa
2012-04-17 9:37 ` Gleb Natapov
2012-04-17 9:44 ` Jiri Olsa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.