* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox