All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] perf parse-events: Fix parsing of >30kb event strings
@ 2025-09-05  4:26 Ian Rogers
  2025-09-17 18:05 ` Ian Rogers
  0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2025-09-05  4:26 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, linux-perf-users,
	linux-kernel

Metrics may generate many particularly uncore event references. The
resulting event string may then be >32kb. The parse events lex is
using "%option reject" which stores backtracking state in a buffer
sized at roughtly 30kb. If the event string is larger than this then a
buffer overflow and typically a crash happens.

The need for "%option reject" was for BPF events which were removed in
commit 3d6dfae88917 ("perf parse-events: Remove BPF event
support"). As "%option reject" is both a memory and performance cost
let's remove it and fix the parsing case for event strings being over
~30kb.

Whilst cleaning up "%option reject" make the header files accurately
reflect functions used in the code and tidy up not requiring yywrap.

Measuring on the "PMU JSON event tests" a modest reduction of 0.41%
user time and 0.27% max resident size was observed. More importantly
this change fixes parsing large metrics and event strings.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/parse-events.l | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 2034590eb789..1eaa8dbc26d8 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -5,16 +5,14 @@
 %option stack
 %option bison-locations
 %option yylineno
-%option reject
+%option noyywrap
 
 %{
 #include <errno.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
 #include "parse-events.h"
 #include "parse-events-bison.h"
-#include "evsel.h"
 
 char *parse_events_get_text(yyscan_t yyscanner);
 YYSTYPE *parse_events_get_lval(yyscan_t yyscanner);
@@ -222,10 +220,6 @@ do {							\
 	yycolumn += yyleng;				\
 } while (0);
 
-#define USER_REJECT		\
-	yycolumn -= yyleng;	\
-	REJECT
-
 %}
 
 %x mem
@@ -423,8 +417,3 @@ r{num_raw_hex}		{ return str(yyscanner, PE_RAW); }
 .			{ }
 
 %%
-
-int parse_events_wrap(void *scanner __maybe_unused)
-{
-	return 1;
-}
-- 
2.51.0.355.g5224444f11-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] perf parse-events: Fix parsing of >30kb event strings
  2025-09-05  4:26 [PATCH v1] perf parse-events: Fix parsing of >30kb event strings Ian Rogers
@ 2025-09-17 18:05 ` Ian Rogers
  2025-10-03 21:25   ` Ian Rogers
  0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2025-09-17 18:05 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, linux-perf-users,
	linux-kernel

On Thu, Sep 4, 2025 at 9:26 PM Ian Rogers <irogers@google.com> wrote:
>
> Metrics may generate many particularly uncore event references. The
> resulting event string may then be >32kb. The parse events lex is
> using "%option reject" which stores backtracking state in a buffer
> sized at roughtly 30kb. If the event string is larger than this then a
> buffer overflow and typically a crash happens.
>
> The need for "%option reject" was for BPF events which were removed in
> commit 3d6dfae88917 ("perf parse-events: Remove BPF event
> support"). As "%option reject" is both a memory and performance cost
> let's remove it and fix the parsing case for event strings being over
> ~30kb.
>
> Whilst cleaning up "%option reject" make the header files accurately
> reflect functions used in the code and tidy up not requiring yywrap.
>
> Measuring on the "PMU JSON event tests" a modest reduction of 0.41%
> user time and 0.27% max resident size was observed. More importantly
> this change fixes parsing large metrics and event strings.
>
> Signed-off-by: Ian Rogers <irogers@google.com>

Ping. I think this may have gotten lost in noise about things like
hardware json and the python metrics. It is a small change, bug fix
and performance win. It can land independently of anything else. PTAL.

Thanks,
Ian

> ---
>  tools/perf/util/parse-events.l | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 2034590eb789..1eaa8dbc26d8 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -5,16 +5,14 @@
>  %option stack
>  %option bison-locations
>  %option yylineno
> -%option reject
> +%option noyywrap
>
>  %{
>  #include <errno.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdio.h>
>  #include "parse-events.h"
>  #include "parse-events-bison.h"
> -#include "evsel.h"
>
>  char *parse_events_get_text(yyscan_t yyscanner);
>  YYSTYPE *parse_events_get_lval(yyscan_t yyscanner);
> @@ -222,10 +220,6 @@ do {                                                       \
>         yycolumn += yyleng;                             \
>  } while (0);
>
> -#define USER_REJECT            \
> -       yycolumn -= yyleng;     \
> -       REJECT
> -
>  %}
>
>  %x mem
> @@ -423,8 +417,3 @@ r{num_raw_hex}              { return str(yyscanner, PE_RAW); }
>  .                      { }
>
>  %%
> -
> -int parse_events_wrap(void *scanner __maybe_unused)
> -{
> -       return 1;
> -}
> --
> 2.51.0.355.g5224444f11-goog
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] perf parse-events: Fix parsing of >30kb event strings
  2025-09-17 18:05 ` Ian Rogers
@ 2025-10-03 21:25   ` Ian Rogers
  2025-10-04 12:58     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2025-10-03 21:25 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, linux-perf-users,
	linux-kernel

On Wed, Sep 17, 2025 at 11:05 AM Ian Rogers <irogers@google.com> wrote:
>
> On Thu, Sep 4, 2025 at 9:26 PM Ian Rogers <irogers@google.com> wrote:
> >
> > Metrics may generate many particularly uncore event references. The
> > resulting event string may then be >32kb. The parse events lex is
> > using "%option reject" which stores backtracking state in a buffer
> > sized at roughtly 30kb. If the event string is larger than this then a
> > buffer overflow and typically a crash happens.
> >
> > The need for "%option reject" was for BPF events which were removed in
> > commit 3d6dfae88917 ("perf parse-events: Remove BPF event
> > support"). As "%option reject" is both a memory and performance cost
> > let's remove it and fix the parsing case for event strings being over
> > ~30kb.
> >
> > Whilst cleaning up "%option reject" make the header files accurately
> > reflect functions used in the code and tidy up not requiring yywrap.
> >
> > Measuring on the "PMU JSON event tests" a modest reduction of 0.41%
> > user time and 0.27% max resident size was observed. More importantly
> > this change fixes parsing large metrics and event strings.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> Ping. I think this may have gotten lost in noise about things like
> hardware json and the python metrics. It is a small change, bug fix
> and performance win. It can land independently of anything else. PTAL.

It'd be nice to land this for the v6.18 pull. I don't think it has any
visible implications and just makes stuff better.

Thanks,
Ian


> Thanks,
> Ian
>
> > ---
> >  tools/perf/util/parse-events.l | 17 +++--------------
> >  1 file changed, 3 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> > index 2034590eb789..1eaa8dbc26d8 100644
> > --- a/tools/perf/util/parse-events.l
> > +++ b/tools/perf/util/parse-events.l
> > @@ -5,16 +5,14 @@
> >  %option stack
> >  %option bison-locations
> >  %option yylineno
> > -%option reject
> > +%option noyywrap
> >
> >  %{
> >  #include <errno.h>
> > -#include <sys/types.h>
> > -#include <sys/stat.h>
> > -#include <unistd.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> >  #include "parse-events.h"
> >  #include "parse-events-bison.h"
> > -#include "evsel.h"
> >
> >  char *parse_events_get_text(yyscan_t yyscanner);
> >  YYSTYPE *parse_events_get_lval(yyscan_t yyscanner);
> > @@ -222,10 +220,6 @@ do {                                                       \
> >         yycolumn += yyleng;                             \
> >  } while (0);
> >
> > -#define USER_REJECT            \
> > -       yycolumn -= yyleng;     \
> > -       REJECT
> > -
> >  %}
> >
> >  %x mem
> > @@ -423,8 +417,3 @@ r{num_raw_hex}              { return str(yyscanner, PE_RAW); }
> >  .                      { }
> >
> >  %%
> > -
> > -int parse_events_wrap(void *scanner __maybe_unused)
> > -{
> > -       return 1;
> > -}
> > --
> > 2.51.0.355.g5224444f11-goog
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] perf parse-events: Fix parsing of >30kb event strings
  2025-10-03 21:25   ` Ian Rogers
@ 2025-10-04 12:58     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-10-04 12:58 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
	linux-perf-users, linux-kernel

On Fri, Oct 03, 2025 at 02:25:39PM -0700, Ian Rogers wrote:
> On Wed, Sep 17, 2025 at 11:05 AM Ian Rogers <irogers@google.com> wrote:
> > On Thu, Sep 4, 2025 at 9:26 PM Ian Rogers <irogers@google.com> wrote:
> > > Whilst cleaning up "%option reject" make the header files accurately
> > > reflect functions used in the code and tidy up not requiring yywrap.

> > > Measuring on the "PMU JSON event tests" a modest reduction of 0.41%
> > > user time and 0.27% max resident size was observed. More importantly
> > > this change fixes parsing large metrics and event strings.

> > Ping. I think this may have gotten lost in noise about things like
> > hardware json and the python metrics. It is a small change, bug fix
> > and performance win. It can land independently of anything else. PTAL.
 
> It'd be nice to land this for the v6.18 pull. I don't think it has any
> visible implications and just makes stuff better.

Applied and now testing on the container build set.

- Arnaldo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-10-04 12:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-05  4:26 [PATCH v1] perf parse-events: Fix parsing of >30kb event strings Ian Rogers
2025-09-17 18:05 ` Ian Rogers
2025-10-03 21:25   ` Ian Rogers
2025-10-04 12:58     ` Arnaldo Carvalho de Melo

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.