* [PATCH v2] perf pmu: Make parser reentrant
@ 2023-04-04 13:36 Ian Rogers
2023-04-05 7:25 ` Jiri Olsa
2023-04-05 21:23 ` Jiri Olsa
0 siblings, 2 replies; 5+ messages in thread
From: Ian Rogers @ 2023-04-04 13:36 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Adrian Hunter, James Clark, Suzuki Poulouse,
Sean Christopherson, Ravi Bangoria, Rob Herring, Leo Yan,
German Gomez, Jing Zhang, Gaosheng Cui, linux-perf-users,
linux-kernel
By default bison uses global state for compatibility with yacc. Make
the parser reentrant so that it may be used in asynchronous and
multithreaded situations.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/pmu.c | 24 ++++++++++++++++++------
tools/perf/util/pmu.h | 2 +-
tools/perf/util/pmu.l | 17 ++++++++++++-----
tools/perf/util/pmu.y | 5 ++++-
4 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 78a407b42ad1..f603cdabf797 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -24,6 +24,8 @@
#include "evsel.h"
#include "pmu.h"
#include "pmus.h"
+#include "pmu-bison.h"
+#include "pmu-flex.h"
#include "parse-events.h"
#include "print-events.h"
#include "header.h"
@@ -57,9 +59,6 @@ struct perf_pmu_format {
struct list_head list;
};
-int perf_pmu_parse(struct list_head *list, char *name);
-extern FILE *perf_pmu_in;
-
static bool hybrid_scanned;
static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name);
@@ -81,6 +80,8 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
while (!ret && (evt_ent = readdir(format_dir))) {
char *name = evt_ent->d_name;
int fd;
+ void *scanner;
+ FILE *file;
if (!strcmp(name, ".") || !strcmp(name, ".."))
continue;
@@ -91,9 +92,20 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
if (fd < 0)
break;
- perf_pmu_in = fdopen(fd, "r");
- ret = perf_pmu_parse(head, name);
- fclose(perf_pmu_in);
+ file = fdopen(fd, "r");
+ if (!file)
+ break;
+
+ ret = perf_pmu_lex_init(&scanner);
+ if (ret) {
+ fclose(file);
+ break;
+ }
+
+ perf_pmu_set_in(file, scanner);
+ ret = perf_pmu_parse(head, name, scanner);
+ perf_pmu_lex_destroy(scanner);
+ fclose(file);
}
closedir(format_dir);
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 32c3a75bca0e..d53618c65c92 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -206,7 +206,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
struct perf_pmu_info *info);
struct list_head *perf_pmu__alias(struct perf_pmu *pmu,
struct list_head *head_terms);
-void perf_pmu_error(struct list_head *list, char *name, char const *msg);
+void perf_pmu_error(struct list_head *list, char *name, void *scanner, char const *msg);
int perf_pmu__new_format(struct list_head *list, char *name,
int config, unsigned long *bits);
diff --git a/tools/perf/util/pmu.l b/tools/perf/util/pmu.l
index 58b4926cfaca..67b247be693b 100644
--- a/tools/perf/util/pmu.l
+++ b/tools/perf/util/pmu.l
@@ -1,4 +1,6 @@
%option prefix="perf_pmu_"
+%option reentrant
+%option bison-bridge
%{
#include <stdlib.h>
@@ -6,16 +8,21 @@
#include "pmu.h"
#include "pmu-bison.h"
-static int value(int base)
+char *perf_pmu_get_text(yyscan_t yyscanner);
+YYSTYPE *perf_pmu_get_lval(yyscan_t yyscanner);
+
+static int value(yyscan_t scanner, int base)
{
+ YYSTYPE *yylval = perf_pmu_get_lval(scanner);
+ char *text = perf_pmu_get_text(scanner);
long num;
errno = 0;
- num = strtoul(perf_pmu_text, NULL, base);
+ num = strtoul(text, NULL, base);
if (errno)
return PP_ERROR;
- perf_pmu_lval.num = num;
+ yylval->num = num;
return PP_VALUE;
}
@@ -25,7 +32,7 @@ num_dec [0-9]+
%%
-{num_dec} { return value(10); }
+{num_dec} { return value(yyscanner, 10); }
config { return PP_CONFIG; }
- { return '-'; }
: { return ':'; }
@@ -35,7 +42,7 @@ config { return PP_CONFIG; }
%%
-int perf_pmu_wrap(void)
+int perf_pmu_wrap(void *scanner __maybe_unused)
{
return 1;
}
diff --git a/tools/perf/util/pmu.y b/tools/perf/util/pmu.y
index e675d79a0274..dff4e892ac4d 100644
--- a/tools/perf/util/pmu.y
+++ b/tools/perf/util/pmu.y
@@ -1,6 +1,8 @@
-
+%define api.pure full
%parse-param {struct list_head *format}
%parse-param {char *name}
+%parse-param {void *scanner}
+%lex-param {void* scanner}
%{
@@ -78,6 +80,7 @@ PP_VALUE
void perf_pmu_error(struct list_head *list __maybe_unused,
char *name __maybe_unused,
+ void *scanner __maybe_unused,
char const *msg __maybe_unused)
{
}
--
2.40.0.348.gf938b09366-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf pmu: Make parser reentrant
2023-04-04 13:36 [PATCH v2] perf pmu: Make parser reentrant Ian Rogers
@ 2023-04-05 7:25 ` Jiri Olsa
2023-04-05 9:39 ` Arnaldo Carvalho de Melo
2023-04-05 21:23 ` Jiri Olsa
1 sibling, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2023-04-05 7:25 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, Adrian Hunter,
James Clark, Suzuki Poulouse, Sean Christopherson, Ravi Bangoria,
Rob Herring, Leo Yan, German Gomez, Jing Zhang, Gaosheng Cui,
linux-perf-users, linux-kernel
On Tue, Apr 04, 2023 at 06:36:30AM -0700, Ian Rogers wrote:
> By default bison uses global state for compatibility with yacc. Make
> the parser reentrant so that it may be used in asynchronous and
> multithreaded situations.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
hum I can't apply this version on Arnaldo's perf/core:
patching file tools/perf/util/pmu.c
Hunk #2 succeeded at 59 with fuzz 1.
Hunk #3 FAILED at 80.
Hunk #4 FAILED at 90.
2 out of 4 hunks FAILED -- saving rejects to file tools/perf/util/pmu.c.rej
patching file tools/perf/util/pmu.h
patching file tools/perf/util/pmu.l
patching file tools/perf/util/pmu.y
jirka
> ---
> tools/perf/util/pmu.c | 24 ++++++++++++++++++------
> tools/perf/util/pmu.h | 2 +-
> tools/perf/util/pmu.l | 17 ++++++++++++-----
> tools/perf/util/pmu.y | 5 ++++-
> 4 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 78a407b42ad1..f603cdabf797 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -24,6 +24,8 @@
> #include "evsel.h"
> #include "pmu.h"
> #include "pmus.h"
> +#include "pmu-bison.h"
> +#include "pmu-flex.h"
> #include "parse-events.h"
> #include "print-events.h"
> #include "header.h"
> @@ -57,9 +59,6 @@ struct perf_pmu_format {
> struct list_head list;
> };
>
> -int perf_pmu_parse(struct list_head *list, char *name);
> -extern FILE *perf_pmu_in;
> -
> static bool hybrid_scanned;
>
> static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name);
> @@ -81,6 +80,8 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
> while (!ret && (evt_ent = readdir(format_dir))) {
> char *name = evt_ent->d_name;
> int fd;
> + void *scanner;
> + FILE *file;
>
> if (!strcmp(name, ".") || !strcmp(name, ".."))
> continue;
> @@ -91,9 +92,20 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
> if (fd < 0)
> break;
>
> - perf_pmu_in = fdopen(fd, "r");
> - ret = perf_pmu_parse(head, name);
> - fclose(perf_pmu_in);
> + file = fdopen(fd, "r");
> + if (!file)
> + break;
> +
> + ret = perf_pmu_lex_init(&scanner);
> + if (ret) {
> + fclose(file);
> + break;
> + }
> +
> + perf_pmu_set_in(file, scanner);
> + ret = perf_pmu_parse(head, name, scanner);
> + perf_pmu_lex_destroy(scanner);
> + fclose(file);
> }
>
> closedir(format_dir);
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 32c3a75bca0e..d53618c65c92 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -206,7 +206,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
> struct perf_pmu_info *info);
> struct list_head *perf_pmu__alias(struct perf_pmu *pmu,
> struct list_head *head_terms);
> -void perf_pmu_error(struct list_head *list, char *name, char const *msg);
> +void perf_pmu_error(struct list_head *list, char *name, void *scanner, char const *msg);
>
> int perf_pmu__new_format(struct list_head *list, char *name,
> int config, unsigned long *bits);
> diff --git a/tools/perf/util/pmu.l b/tools/perf/util/pmu.l
> index 58b4926cfaca..67b247be693b 100644
> --- a/tools/perf/util/pmu.l
> +++ b/tools/perf/util/pmu.l
> @@ -1,4 +1,6 @@
> %option prefix="perf_pmu_"
> +%option reentrant
> +%option bison-bridge
>
> %{
> #include <stdlib.h>
> @@ -6,16 +8,21 @@
> #include "pmu.h"
> #include "pmu-bison.h"
>
> -static int value(int base)
> +char *perf_pmu_get_text(yyscan_t yyscanner);
> +YYSTYPE *perf_pmu_get_lval(yyscan_t yyscanner);
> +
> +static int value(yyscan_t scanner, int base)
> {
> + YYSTYPE *yylval = perf_pmu_get_lval(scanner);
> + char *text = perf_pmu_get_text(scanner);
> long num;
>
> errno = 0;
> - num = strtoul(perf_pmu_text, NULL, base);
> + num = strtoul(text, NULL, base);
> if (errno)
> return PP_ERROR;
>
> - perf_pmu_lval.num = num;
> + yylval->num = num;
> return PP_VALUE;
> }
>
> @@ -25,7 +32,7 @@ num_dec [0-9]+
>
> %%
>
> -{num_dec} { return value(10); }
> +{num_dec} { return value(yyscanner, 10); }
> config { return PP_CONFIG; }
> - { return '-'; }
> : { return ':'; }
> @@ -35,7 +42,7 @@ config { return PP_CONFIG; }
>
> %%
>
> -int perf_pmu_wrap(void)
> +int perf_pmu_wrap(void *scanner __maybe_unused)
> {
> return 1;
> }
> diff --git a/tools/perf/util/pmu.y b/tools/perf/util/pmu.y
> index e675d79a0274..dff4e892ac4d 100644
> --- a/tools/perf/util/pmu.y
> +++ b/tools/perf/util/pmu.y
> @@ -1,6 +1,8 @@
> -
> +%define api.pure full
> %parse-param {struct list_head *format}
> %parse-param {char *name}
> +%parse-param {void *scanner}
> +%lex-param {void* scanner}
>
> %{
>
> @@ -78,6 +80,7 @@ PP_VALUE
>
> void perf_pmu_error(struct list_head *list __maybe_unused,
> char *name __maybe_unused,
> + void *scanner __maybe_unused,
> char const *msg __maybe_unused)
> {
> }
> --
> 2.40.0.348.gf938b09366-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf pmu: Make parser reentrant
2023-04-05 7:25 ` Jiri Olsa
@ 2023-04-05 9:39 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-04-05 9:39 UTC (permalink / raw)
To: Jiri Olsa, Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, Adrian Hunter,
James Clark, Suzuki Poulouse, Sean Christopherson, Ravi Bangoria,
Rob Herring, Leo Yan, German Gomez, Jing Zhang, Gaosheng Cui,
linux-perf-users, linux-kernel
On April 5, 2023 4:25:18 AM GMT-03:00, Jiri Olsa <olsajiri@gmail.com> wrote:
>On Tue, Apr 04, 2023 at 06:36:30AM -0700, Ian Rogers wrote:
>> By default bison uses global state for compatibility with yacc. Make
>> the parser reentrant so that it may be used in asynchronous and
>> multithreaded situations.
>>
>> Signed-off-by: Ian Rogers <irogers@google.com>
>
>hum I can't apply this version on Arnaldo's perf/core:
Try on tmp.perf-tools-next
>
>patching file tools/perf/util/pmu.c
>Hunk #2 succeeded at 59 with fuzz 1.
>Hunk #3 FAILED at 80.
>Hunk #4 FAILED at 90.
>2 out of 4 hunks FAILED -- saving rejects to file tools/perf/util/pmu.c.rej
>patching file tools/perf/util/pmu.h
>patching file tools/perf/util/pmu.l
>patching file tools/perf/util/pmu.y
>
>jirka
>
>
>> ---
>> tools/perf/util/pmu.c | 24 ++++++++++++++++++------
>> tools/perf/util/pmu.h | 2 +-
>> tools/perf/util/pmu.l | 17 ++++++++++++-----
>> tools/perf/util/pmu.y | 5 ++++-
>> 4 files changed, 35 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
>> index 78a407b42ad1..f603cdabf797 100644
>> --- a/tools/perf/util/pmu.c
>> +++ b/tools/perf/util/pmu.c
>> @@ -24,6 +24,8 @@
>> #include "evsel.h"
>> #include "pmu.h"
>> #include "pmus.h"
>> +#include "pmu-bison.h"
>> +#include "pmu-flex.h"
>> #include "parse-events.h"
>> #include "print-events.h"
>> #include "header.h"
>> @@ -57,9 +59,6 @@ struct perf_pmu_format {
>> struct list_head list;
>> };
>>
>> -int perf_pmu_parse(struct list_head *list, char *name);
>> -extern FILE *perf_pmu_in;
>> -
>> static bool hybrid_scanned;
>>
>> static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name);
>> @@ -81,6 +80,8 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
>> while (!ret && (evt_ent = readdir(format_dir))) {
>> char *name = evt_ent->d_name;
>> int fd;
>> + void *scanner;
>> + FILE *file;
>>
>> if (!strcmp(name, ".") || !strcmp(name, ".."))
>> continue;
>> @@ -91,9 +92,20 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
>> if (fd < 0)
>> break;
>>
>> - perf_pmu_in = fdopen(fd, "r");
>> - ret = perf_pmu_parse(head, name);
>> - fclose(perf_pmu_in);
>> + file = fdopen(fd, "r");
>> + if (!file)
>> + break;
>> +
>> + ret = perf_pmu_lex_init(&scanner);
>> + if (ret) {
>> + fclose(file);
>> + break;
>> + }
>> +
>> + perf_pmu_set_in(file, scanner);
>> + ret = perf_pmu_parse(head, name, scanner);
>> + perf_pmu_lex_destroy(scanner);
>> + fclose(file);
>> }
>>
>> closedir(format_dir);
>> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
>> index 32c3a75bca0e..d53618c65c92 100644
>> --- a/tools/perf/util/pmu.h
>> +++ b/tools/perf/util/pmu.h
>> @@ -206,7 +206,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
>> struct perf_pmu_info *info);
>> struct list_head *perf_pmu__alias(struct perf_pmu *pmu,
>> struct list_head *head_terms);
>> -void perf_pmu_error(struct list_head *list, char *name, char const *msg);
>> +void perf_pmu_error(struct list_head *list, char *name, void *scanner, char const *msg);
>>
>> int perf_pmu__new_format(struct list_head *list, char *name,
>> int config, unsigned long *bits);
>> diff --git a/tools/perf/util/pmu.l b/tools/perf/util/pmu.l
>> index 58b4926cfaca..67b247be693b 100644
>> --- a/tools/perf/util/pmu.l
>> +++ b/tools/perf/util/pmu.l
>> @@ -1,4 +1,6 @@
>> %option prefix="perf_pmu_"
>> +%option reentrant
>> +%option bison-bridge
>>
>> %{
>> #include <stdlib.h>
>> @@ -6,16 +8,21 @@
>> #include "pmu.h"
>> #include "pmu-bison.h"
>>
>> -static int value(int base)
>> +char *perf_pmu_get_text(yyscan_t yyscanner);
>> +YYSTYPE *perf_pmu_get_lval(yyscan_t yyscanner);
>> +
>> +static int value(yyscan_t scanner, int base)
>> {
>> + YYSTYPE *yylval = perf_pmu_get_lval(scanner);
>> + char *text = perf_pmu_get_text(scanner);
>> long num;
>>
>> errno = 0;
>> - num = strtoul(perf_pmu_text, NULL, base);
>> + num = strtoul(text, NULL, base);
>> if (errno)
>> return PP_ERROR;
>>
>> - perf_pmu_lval.num = num;
>> + yylval->num = num;
>> return PP_VALUE;
>> }
>>
>> @@ -25,7 +32,7 @@ num_dec [0-9]+
>>
>> %%
>>
>> -{num_dec} { return value(10); }
>> +{num_dec} { return value(yyscanner, 10); }
>> config { return PP_CONFIG; }
>> - { return '-'; }
>> : { return ':'; }
>> @@ -35,7 +42,7 @@ config { return PP_CONFIG; }
>>
>> %%
>>
>> -int perf_pmu_wrap(void)
>> +int perf_pmu_wrap(void *scanner __maybe_unused)
>> {
>> return 1;
>> }
>> diff --git a/tools/perf/util/pmu.y b/tools/perf/util/pmu.y
>> index e675d79a0274..dff4e892ac4d 100644
>> --- a/tools/perf/util/pmu.y
>> +++ b/tools/perf/util/pmu.y
>> @@ -1,6 +1,8 @@
>> -
>> +%define api.pure full
>> %parse-param {struct list_head *format}
>> %parse-param {char *name}
>> +%parse-param {void *scanner}
>> +%lex-param {void* scanner}
>>
>> %{
>>
>> @@ -78,6 +80,7 @@ PP_VALUE
>>
>> void perf_pmu_error(struct list_head *list __maybe_unused,
>> char *name __maybe_unused,
>> + void *scanner __maybe_unused,
>> char const *msg __maybe_unused)
>> {
>> }
>> --
>> 2.40.0.348.gf938b09366-goog
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf pmu: Make parser reentrant
2023-04-04 13:36 [PATCH v2] perf pmu: Make parser reentrant Ian Rogers
2023-04-05 7:25 ` Jiri Olsa
@ 2023-04-05 21:23 ` Jiri Olsa
2023-04-06 6:20 ` Ian Rogers
1 sibling, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2023-04-05 21:23 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, Adrian Hunter,
James Clark, Suzuki Poulouse, Sean Christopherson, Ravi Bangoria,
Rob Herring, Leo Yan, German Gomez, Jing Zhang, Gaosheng Cui,
linux-perf-users, linux-kernel
On Tue, Apr 04, 2023 at 06:36:30AM -0700, Ian Rogers wrote:
> By default bison uses global state for compatibility with yacc. Make
> the parser reentrant so that it may be used in asynchronous and
> multithreaded situations.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/pmu.c | 24 ++++++++++++++++++------
> tools/perf/util/pmu.h | 2 +-
> tools/perf/util/pmu.l | 17 ++++++++++++-----
> tools/perf/util/pmu.y | 5 ++++-
> 4 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 78a407b42ad1..f603cdabf797 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -24,6 +24,8 @@
> #include "evsel.h"
> #include "pmu.h"
> #include "pmus.h"
> +#include "pmu-bison.h"
> +#include "pmu-flex.h"
> #include "parse-events.h"
> #include "print-events.h"
> #include "header.h"
> @@ -57,9 +59,6 @@ struct perf_pmu_format {
> struct list_head list;
> };
>
> -int perf_pmu_parse(struct list_head *list, char *name);
> -extern FILE *perf_pmu_in;
> -
> static bool hybrid_scanned;
>
> static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name);
> @@ -81,6 +80,8 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
> while (!ret && (evt_ent = readdir(format_dir))) {
> char *name = evt_ent->d_name;
> int fd;
> + void *scanner;
> + FILE *file;
>
> if (!strcmp(name, ".") || !strcmp(name, ".."))
> continue;
> @@ -91,9 +92,20 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
> if (fd < 0)
> break;
>
> - perf_pmu_in = fdopen(fd, "r");
> - ret = perf_pmu_parse(head, name);
> - fclose(perf_pmu_in);
> + file = fdopen(fd, "r");
> + if (!file)
> + break;
hum, do we potentially leak fd in here?
jirka
> +
> + ret = perf_pmu_lex_init(&scanner);
> + if (ret) {
> + fclose(file);
> + break;
> + }
> +
> + perf_pmu_set_in(file, scanner);
> + ret = perf_pmu_parse(head, name, scanner);
> + perf_pmu_lex_destroy(scanner);
> + fclose(file);
> }
>
> closedir(format_dir);
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 32c3a75bca0e..d53618c65c92 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -206,7 +206,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
> struct perf_pmu_info *info);
> struct list_head *perf_pmu__alias(struct perf_pmu *pmu,
> struct list_head *head_terms);
> -void perf_pmu_error(struct list_head *list, char *name, char const *msg);
> +void perf_pmu_error(struct list_head *list, char *name, void *scanner, char const *msg);
>
> int perf_pmu__new_format(struct list_head *list, char *name,
> int config, unsigned long *bits);
> diff --git a/tools/perf/util/pmu.l b/tools/perf/util/pmu.l
> index 58b4926cfaca..67b247be693b 100644
> --- a/tools/perf/util/pmu.l
> +++ b/tools/perf/util/pmu.l
> @@ -1,4 +1,6 @@
> %option prefix="perf_pmu_"
> +%option reentrant
> +%option bison-bridge
>
> %{
> #include <stdlib.h>
> @@ -6,16 +8,21 @@
> #include "pmu.h"
> #include "pmu-bison.h"
>
> -static int value(int base)
> +char *perf_pmu_get_text(yyscan_t yyscanner);
> +YYSTYPE *perf_pmu_get_lval(yyscan_t yyscanner);
> +
> +static int value(yyscan_t scanner, int base)
> {
> + YYSTYPE *yylval = perf_pmu_get_lval(scanner);
> + char *text = perf_pmu_get_text(scanner);
> long num;
>
> errno = 0;
> - num = strtoul(perf_pmu_text, NULL, base);
> + num = strtoul(text, NULL, base);
> if (errno)
> return PP_ERROR;
>
> - perf_pmu_lval.num = num;
> + yylval->num = num;
> return PP_VALUE;
> }
>
> @@ -25,7 +32,7 @@ num_dec [0-9]+
>
> %%
>
> -{num_dec} { return value(10); }
> +{num_dec} { return value(yyscanner, 10); }
> config { return PP_CONFIG; }
> - { return '-'; }
> : { return ':'; }
> @@ -35,7 +42,7 @@ config { return PP_CONFIG; }
>
> %%
>
> -int perf_pmu_wrap(void)
> +int perf_pmu_wrap(void *scanner __maybe_unused)
> {
> return 1;
> }
> diff --git a/tools/perf/util/pmu.y b/tools/perf/util/pmu.y
> index e675d79a0274..dff4e892ac4d 100644
> --- a/tools/perf/util/pmu.y
> +++ b/tools/perf/util/pmu.y
> @@ -1,6 +1,8 @@
> -
> +%define api.pure full
> %parse-param {struct list_head *format}
> %parse-param {char *name}
> +%parse-param {void *scanner}
> +%lex-param {void* scanner}
>
> %{
>
> @@ -78,6 +80,7 @@ PP_VALUE
>
> void perf_pmu_error(struct list_head *list __maybe_unused,
> char *name __maybe_unused,
> + void *scanner __maybe_unused,
> char const *msg __maybe_unused)
> {
> }
> --
> 2.40.0.348.gf938b09366-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf pmu: Make parser reentrant
2023-04-05 21:23 ` Jiri Olsa
@ 2023-04-06 6:20 ` Ian Rogers
0 siblings, 0 replies; 5+ messages in thread
From: Ian Rogers @ 2023-04-06 6:20 UTC (permalink / raw)
To: Jiri Olsa
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, Adrian Hunter,
James Clark, Suzuki Poulouse, Sean Christopherson, Ravi Bangoria,
Rob Herring, Leo Yan, German Gomez, Jing Zhang, Gaosheng Cui,
linux-perf-users, linux-kernel
On Wed, Apr 5, 2023 at 2:23 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Apr 04, 2023 at 06:36:30AM -0700, Ian Rogers wrote:
> > By default bison uses global state for compatibility with yacc. Make
> > the parser reentrant so that it may be used in asynchronous and
> > multithreaded situations.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/util/pmu.c | 24 ++++++++++++++++++------
> > tools/perf/util/pmu.h | 2 +-
> > tools/perf/util/pmu.l | 17 ++++++++++++-----
> > tools/perf/util/pmu.y | 5 ++++-
> > 4 files changed, 35 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index 78a407b42ad1..f603cdabf797 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -24,6 +24,8 @@
> > #include "evsel.h"
> > #include "pmu.h"
> > #include "pmus.h"
> > +#include "pmu-bison.h"
> > +#include "pmu-flex.h"
> > #include "parse-events.h"
> > #include "print-events.h"
> > #include "header.h"
> > @@ -57,9 +59,6 @@ struct perf_pmu_format {
> > struct list_head list;
> > };
> >
> > -int perf_pmu_parse(struct list_head *list, char *name);
> > -extern FILE *perf_pmu_in;
> > -
> > static bool hybrid_scanned;
> >
> > static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name);
> > @@ -81,6 +80,8 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
> > while (!ret && (evt_ent = readdir(format_dir))) {
> > char *name = evt_ent->d_name;
> > int fd;
> > + void *scanner;
> > + FILE *file;
> >
> > if (!strcmp(name, ".") || !strcmp(name, ".."))
> > continue;
> > @@ -91,9 +92,20 @@ int perf_pmu__format_parse(int dirfd, struct list_head *head)
> > if (fd < 0)
> > break;
> >
> > - perf_pmu_in = fdopen(fd, "r");
> > - ret = perf_pmu_parse(head, name);
> > - fclose(perf_pmu_in);
> > + file = fdopen(fd, "r");
> > + if (!file)
> > + break;
>
> hum, do we potentially leak fd in here?
>
> jirka
Agreed. Will fix in v3.
Thanks,
Ian
> > +
> > + ret = perf_pmu_lex_init(&scanner);
> > + if (ret) {
> > + fclose(file);
> > + break;
> > + }
> > +
> > + perf_pmu_set_in(file, scanner);
> > + ret = perf_pmu_parse(head, name, scanner);
> > + perf_pmu_lex_destroy(scanner);
> > + fclose(file);
> > }
> >
> > closedir(format_dir);
> > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> > index 32c3a75bca0e..d53618c65c92 100644
> > --- a/tools/perf/util/pmu.h
> > +++ b/tools/perf/util/pmu.h
> > @@ -206,7 +206,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
> > struct perf_pmu_info *info);
> > struct list_head *perf_pmu__alias(struct perf_pmu *pmu,
> > struct list_head *head_terms);
> > -void perf_pmu_error(struct list_head *list, char *name, char const *msg);
> > +void perf_pmu_error(struct list_head *list, char *name, void *scanner, char const *msg);
> >
> > int perf_pmu__new_format(struct list_head *list, char *name,
> > int config, unsigned long *bits);
> > diff --git a/tools/perf/util/pmu.l b/tools/perf/util/pmu.l
> > index 58b4926cfaca..67b247be693b 100644
> > --- a/tools/perf/util/pmu.l
> > +++ b/tools/perf/util/pmu.l
> > @@ -1,4 +1,6 @@
> > %option prefix="perf_pmu_"
> > +%option reentrant
> > +%option bison-bridge
> >
> > %{
> > #include <stdlib.h>
> > @@ -6,16 +8,21 @@
> > #include "pmu.h"
> > #include "pmu-bison.h"
> >
> > -static int value(int base)
> > +char *perf_pmu_get_text(yyscan_t yyscanner);
> > +YYSTYPE *perf_pmu_get_lval(yyscan_t yyscanner);
> > +
> > +static int value(yyscan_t scanner, int base)
> > {
> > + YYSTYPE *yylval = perf_pmu_get_lval(scanner);
> > + char *text = perf_pmu_get_text(scanner);
> > long num;
> >
> > errno = 0;
> > - num = strtoul(perf_pmu_text, NULL, base);
> > + num = strtoul(text, NULL, base);
> > if (errno)
> > return PP_ERROR;
> >
> > - perf_pmu_lval.num = num;
> > + yylval->num = num;
> > return PP_VALUE;
> > }
> >
> > @@ -25,7 +32,7 @@ num_dec [0-9]+
> >
> > %%
> >
> > -{num_dec} { return value(10); }
> > +{num_dec} { return value(yyscanner, 10); }
> > config { return PP_CONFIG; }
> > - { return '-'; }
> > : { return ':'; }
> > @@ -35,7 +42,7 @@ config { return PP_CONFIG; }
> >
> > %%
> >
> > -int perf_pmu_wrap(void)
> > +int perf_pmu_wrap(void *scanner __maybe_unused)
> > {
> > return 1;
> > }
> > diff --git a/tools/perf/util/pmu.y b/tools/perf/util/pmu.y
> > index e675d79a0274..dff4e892ac4d 100644
> > --- a/tools/perf/util/pmu.y
> > +++ b/tools/perf/util/pmu.y
> > @@ -1,6 +1,8 @@
> > -
> > +%define api.pure full
> > %parse-param {struct list_head *format}
> > %parse-param {char *name}
> > +%parse-param {void *scanner}
> > +%lex-param {void* scanner}
> >
> > %{
> >
> > @@ -78,6 +80,7 @@ PP_VALUE
> >
> > void perf_pmu_error(struct list_head *list __maybe_unused,
> > char *name __maybe_unused,
> > + void *scanner __maybe_unused,
> > char const *msg __maybe_unused)
> > {
> > }
> > --
> > 2.40.0.348.gf938b09366-goog
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-04-06 6:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-04 13:36 [PATCH v2] perf pmu: Make parser reentrant Ian Rogers
2023-04-05 7:25 ` Jiri Olsa
2023-04-05 9:39 ` Arnaldo Carvalho de Melo
2023-04-05 21:23 ` Jiri Olsa
2023-04-06 6:20 ` Ian Rogers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).