linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once
@ 2023-06-01  2:36 Ian Rogers
  2023-06-01  2:36 ` [PATCH v1 2/2] perf pmu: Warn about invalid config for all PMUs and configs Ian Rogers
  2023-06-01 20:04 ` [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once Namhyung Kim
  0 siblings, 2 replies; 5+ messages in thread
From: Ian Rogers @ 2023-06-01  2: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, Kan Liang, Rob Herring, Zhengjun Xing,
	James Clark, Suzuki Poulouse, linux-perf-users, linux-kernel

Avoid scanning format list for each event parsed.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/pmu.c | 5 +++++
 tools/perf/util/pmu.h | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 0520aa9fe991..204ce3f02e63 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -934,6 +934,11 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
 {
 	struct perf_pmu_format *format;
 
+	if (pmu->formats_checked)
+		return;
+
+	pmu->formats_checked = true;
+
 	/* fake pmu doesn't have format list */
 	if (pmu == &perf_pmu__fake)
 		return;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 287f593b15c7..7a1535dc1f12 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -76,6 +76,11 @@ struct perf_pmu {
 	 * specific code.
 	 */
 	bool auxtrace;
+	/**
+	 * @formats_checked: Only check PMU's formats are valid for
+	 * perf_event_attr once.
+	 */
+	bool formats_checked;
 	/**
 	 * @max_precise: Number of levels of :ppp precision supported by the
 	 * PMU, read from
-- 
2.41.0.rc0.172.g3f132b7071-goog


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

* [PATCH v1 2/2] perf pmu: Warn about invalid config for all PMUs and configs
  2023-06-01  2:36 [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once Ian Rogers
@ 2023-06-01  2:36 ` Ian Rogers
  2023-06-01 20:05   ` Namhyung Kim
  2023-06-01 20:04 ` [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once Namhyung Kim
  1 sibling, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2023-06-01  2: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, Kan Liang, Rob Herring, Zhengjun Xing,
	James Clark, Suzuki Poulouse, linux-perf-users, linux-kernel

Don't just check the raw PMU type, the only core PMU on homogeneous
x86, check raw and all dynamically added PMUs. Extend the
perf_pmu__warn_invalid_config to check all 4 config values. Rather
than process the format list once per event, store the computed masks
for each config value. Don't ignore the mask being zero, which is
likely for config2 and config3, add config_masks_present so config
values can be ignored only when no format information is present.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/parse-events.c | 13 +++++++++---
 tools/perf/util/pmu.c          | 38 ++++++++++++++++++++++++----------
 tools/perf/util/pmu.h          | 13 +++++++++++-
 3 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 7f047ac11168..9f60607b0d86 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -245,9 +245,16 @@ __add_event(struct list_head *list, int *idx,
 	if (pmu)
 		perf_pmu__warn_invalid_formats(pmu);
 
-	if (pmu && attr->type == PERF_TYPE_RAW)
-		perf_pmu__warn_invalid_config(pmu, attr->config, name);
-
+	if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
+		perf_pmu__warn_invalid_config(pmu, attr->config, name,
+					      PERF_PMU_FORMAT_VALUE_CONFIG, "config");
+		perf_pmu__warn_invalid_config(pmu, attr->config1, name,
+					      PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
+		perf_pmu__warn_invalid_config(pmu, attr->config2, name,
+					      PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
+		perf_pmu__warn_invalid_config(pmu, attr->config3, name,
+					      PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
+	}
 	if (init_attr)
 		event_attr_init(attr);
 
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 204ce3f02e63..b0443406fd57 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1611,37 +1611,53 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
 	return pmu->nr_caps;
 }
 
-void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
-				   const char *name)
+static void perf_pmu__compute_config_masks(struct perf_pmu *pmu)
 {
 	struct perf_pmu_format *format;
-	__u64 masks = 0, bits;
-	char buf[100];
-	unsigned int i;
+
+	if (pmu->config_masks_computed)
+		return;
 
 	list_for_each_entry(format, &pmu->format, list)	{
-		if (format->value != PERF_PMU_FORMAT_VALUE_CONFIG)
+		unsigned int i;
+		__u64 *mask;
+
+		if (format->value >= PERF_PMU_FORMAT_VALUE_CONFIG_END)
 			continue;
 
+		pmu->config_masks_present = true;
+		mask = &pmu->config_masks[format->value];
+
 		for_each_set_bit(i, format->bits, PERF_PMU_FORMAT_BITS)
-			masks |= 1ULL << i;
+			*mask |= 1ULL << i;
 	}
+	pmu->config_masks_computed = true;
+}
+
+void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
+				   const char *name, int config_num,
+				   const char *config_name)
+{
+	__u64 bits;
+	char buf[100];
+
+	perf_pmu__compute_config_masks(pmu);
 
 	/*
 	 * Kernel doesn't export any valid format bits.
 	 */
-	if (masks == 0)
+	if (!pmu->config_masks_present)
 		return;
 
-	bits = config & ~masks;
+	bits = config & ~pmu->config_masks[config_num];
 	if (bits == 0)
 		return;
 
 	bitmap_scnprintf((unsigned long *)&bits, sizeof(bits) * 8, buf, sizeof(buf));
 
-	pr_warning("WARNING: event '%s' not valid (bits %s of config "
+	pr_warning("WARNING: event '%s' not valid (bits %s of %s "
 		   "'%llx' not supported by kernel)!\n",
-		   name ?: "N/A", buf, config);
+		   name ?: "N/A", buf, config_name, config);
 }
 
 int perf_pmu__match(char *pattern, char *name, char *tok)
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 7a1535dc1f12..d98b0feec022 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -81,6 +81,10 @@ struct perf_pmu {
 	 * perf_event_attr once.
 	 */
 	bool formats_checked;
+	/** @config_masks_present: Are there config format values? */
+	bool config_masks_present;
+	/** @config_masks_computed: Set when masks are lazily computed. */
+	bool config_masks_computed;
 	/**
 	 * @max_precise: Number of levels of :ppp precision supported by the
 	 * PMU, read from
@@ -125,6 +129,12 @@ struct perf_pmu {
 	/** @list: Element on pmus list in pmu.c. */
 	struct list_head list;
 
+	/**
+	 * @config_masks: Derived from the PMU's format data, bits that are
+	 * valid within the config value.
+	 */
+	__u64 config_masks[PERF_PMU_FORMAT_VALUE_CONFIG_END];
+
 	/**
 	 * @missing_features: Features to inhibit when events on this PMU are
 	 * opened.
@@ -255,7 +265,8 @@ int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
 int perf_pmu__caps_parse(struct perf_pmu *pmu);
 
 void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
-				   const char *name);
+				   const char *name, int config_num,
+				   const char *config_name);
 void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu);
 
 int perf_pmu__match(char *pattern, char *name, char *tok);
-- 
2.41.0.rc0.172.g3f132b7071-goog


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

* Re: [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once
  2023-06-01  2:36 [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once Ian Rogers
  2023-06-01  2:36 ` [PATCH v1 2/2] perf pmu: Warn about invalid config for all PMUs and configs Ian Rogers
@ 2023-06-01 20:04 ` Namhyung Kim
  2023-06-01 20:19   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2023-06-01 20:04 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Rob Herring, Zhengjun Xing, James Clark,
	Suzuki Poulouse, linux-perf-users, linux-kernel

On Wed, May 31, 2023 at 7:36 PM Ian Rogers <irogers@google.com> wrote:
>
> Avoid scanning format list for each event parsed.

Maybe it's better to change the subject that it's about format..
Other than that,

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/pmu.c | 5 +++++
>  tools/perf/util/pmu.h | 5 +++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 0520aa9fe991..204ce3f02e63 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -934,6 +934,11 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
>  {
>         struct perf_pmu_format *format;
>
> +       if (pmu->formats_checked)
> +               return;
> +
> +       pmu->formats_checked = true;
> +
>         /* fake pmu doesn't have format list */
>         if (pmu == &perf_pmu__fake)
>                 return;
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 287f593b15c7..7a1535dc1f12 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -76,6 +76,11 @@ struct perf_pmu {
>          * specific code.
>          */
>         bool auxtrace;
> +       /**
> +        * @formats_checked: Only check PMU's formats are valid for
> +        * perf_event_attr once.
> +        */
> +       bool formats_checked;
>         /**
>          * @max_precise: Number of levels of :ppp precision supported by the
>          * PMU, read from
> --
> 2.41.0.rc0.172.g3f132b7071-goog
>

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

* Re: [PATCH v1 2/2] perf pmu: Warn about invalid config for all PMUs and configs
  2023-06-01  2:36 ` [PATCH v1 2/2] perf pmu: Warn about invalid config for all PMUs and configs Ian Rogers
@ 2023-06-01 20:05   ` Namhyung Kim
  0 siblings, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2023-06-01 20:05 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Rob Herring, Zhengjun Xing, James Clark,
	Suzuki Poulouse, linux-perf-users, linux-kernel

On Wed, May 31, 2023 at 7:36 PM Ian Rogers <irogers@google.com> wrote:
>
> Don't just check the raw PMU type, the only core PMU on homogeneous
> x86, check raw and all dynamically added PMUs. Extend the
> perf_pmu__warn_invalid_config to check all 4 config values. Rather
> than process the format list once per event, store the computed masks
> for each config value. Don't ignore the mask being zero, which is
> likely for config2 and config3, add config_masks_present so config
> values can be ignored only when no format information is present.
>
> Signed-off-by: Ian Rogers <irogers@google.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/perf/util/parse-events.c | 13 +++++++++---
>  tools/perf/util/pmu.c          | 38 ++++++++++++++++++++++++----------
>  tools/perf/util/pmu.h          | 13 +++++++++++-
>  3 files changed, 49 insertions(+), 15 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 7f047ac11168..9f60607b0d86 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -245,9 +245,16 @@ __add_event(struct list_head *list, int *idx,
>         if (pmu)
>                 perf_pmu__warn_invalid_formats(pmu);
>
> -       if (pmu && attr->type == PERF_TYPE_RAW)
> -               perf_pmu__warn_invalid_config(pmu, attr->config, name);
> -
> +       if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
> +               perf_pmu__warn_invalid_config(pmu, attr->config, name,
> +                                             PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> +               perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> +                                             PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> +               perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> +                                             PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> +               perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> +                                             PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> +       }
>         if (init_attr)
>                 event_attr_init(attr);
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 204ce3f02e63..b0443406fd57 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1611,37 +1611,53 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
>         return pmu->nr_caps;
>  }
>
> -void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> -                                  const char *name)
> +static void perf_pmu__compute_config_masks(struct perf_pmu *pmu)
>  {
>         struct perf_pmu_format *format;
> -       __u64 masks = 0, bits;
> -       char buf[100];
> -       unsigned int i;
> +
> +       if (pmu->config_masks_computed)
> +               return;
>
>         list_for_each_entry(format, &pmu->format, list) {
> -               if (format->value != PERF_PMU_FORMAT_VALUE_CONFIG)
> +               unsigned int i;
> +               __u64 *mask;
> +
> +               if (format->value >= PERF_PMU_FORMAT_VALUE_CONFIG_END)
>                         continue;
>
> +               pmu->config_masks_present = true;
> +               mask = &pmu->config_masks[format->value];
> +
>                 for_each_set_bit(i, format->bits, PERF_PMU_FORMAT_BITS)
> -                       masks |= 1ULL << i;
> +                       *mask |= 1ULL << i;
>         }
> +       pmu->config_masks_computed = true;
> +}
> +
> +void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> +                                  const char *name, int config_num,
> +                                  const char *config_name)
> +{
> +       __u64 bits;
> +       char buf[100];
> +
> +       perf_pmu__compute_config_masks(pmu);
>
>         /*
>          * Kernel doesn't export any valid format bits.
>          */
> -       if (masks == 0)
> +       if (!pmu->config_masks_present)
>                 return;
>
> -       bits = config & ~masks;
> +       bits = config & ~pmu->config_masks[config_num];
>         if (bits == 0)
>                 return;
>
>         bitmap_scnprintf((unsigned long *)&bits, sizeof(bits) * 8, buf, sizeof(buf));
>
> -       pr_warning("WARNING: event '%s' not valid (bits %s of config "
> +       pr_warning("WARNING: event '%s' not valid (bits %s of %s "
>                    "'%llx' not supported by kernel)!\n",
> -                  name ?: "N/A", buf, config);
> +                  name ?: "N/A", buf, config_name, config);
>  }
>
>  int perf_pmu__match(char *pattern, char *name, char *tok)
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 7a1535dc1f12..d98b0feec022 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -81,6 +81,10 @@ struct perf_pmu {
>          * perf_event_attr once.
>          */
>         bool formats_checked;
> +       /** @config_masks_present: Are there config format values? */
> +       bool config_masks_present;
> +       /** @config_masks_computed: Set when masks are lazily computed. */
> +       bool config_masks_computed;
>         /**
>          * @max_precise: Number of levels of :ppp precision supported by the
>          * PMU, read from
> @@ -125,6 +129,12 @@ struct perf_pmu {
>         /** @list: Element on pmus list in pmu.c. */
>         struct list_head list;
>
> +       /**
> +        * @config_masks: Derived from the PMU's format data, bits that are
> +        * valid within the config value.
> +        */
> +       __u64 config_masks[PERF_PMU_FORMAT_VALUE_CONFIG_END];
> +
>         /**
>          * @missing_features: Features to inhibit when events on this PMU are
>          * opened.
> @@ -255,7 +265,8 @@ int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
>  int perf_pmu__caps_parse(struct perf_pmu *pmu);
>
>  void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> -                                  const char *name);
> +                                  const char *name, int config_num,
> +                                  const char *config_name);
>  void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu);
>
>  int perf_pmu__match(char *pattern, char *name, char *tok);
> --
> 2.41.0.rc0.172.g3f132b7071-goog
>

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

* Re: [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once
  2023-06-01 20:04 ` [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once Namhyung Kim
@ 2023-06-01 20:19   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-06-01 20:19 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
	Rob Herring, Zhengjun Xing, James Clark, Suzuki Poulouse,
	linux-perf-users, linux-kernel

Em Thu, Jun 01, 2023 at 01:04:30PM -0700, Namhyung Kim escreveu:
> On Wed, May 31, 2023 at 7:36 PM Ian Rogers <irogers@google.com> wrote:
> >
> > Avoid scanning format list for each event parsed.
> 
> Maybe it's better to change the subject that it's about format..
> Other than that,

Ok, I can fix that, applying.
 
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> 
> Thanks,
> Namhyung
> 
> 
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/util/pmu.c | 5 +++++
> >  tools/perf/util/pmu.h | 5 +++++
> >  2 files changed, 10 insertions(+)
> >
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index 0520aa9fe991..204ce3f02e63 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -934,6 +934,11 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
> >  {
> >         struct perf_pmu_format *format;
> >
> > +       if (pmu->formats_checked)
> > +               return;
> > +
> > +       pmu->formats_checked = true;
> > +
> >         /* fake pmu doesn't have format list */
> >         if (pmu == &perf_pmu__fake)
> >                 return;
> > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> > index 287f593b15c7..7a1535dc1f12 100644
> > --- a/tools/perf/util/pmu.h
> > +++ b/tools/perf/util/pmu.h
> > @@ -76,6 +76,11 @@ struct perf_pmu {
> >          * specific code.
> >          */
> >         bool auxtrace;
> > +       /**
> > +        * @formats_checked: Only check PMU's formats are valid for
> > +        * perf_event_attr once.
> > +        */
> > +       bool formats_checked;
> >         /**
> >          * @max_precise: Number of levels of :ppp precision supported by the
> >          * PMU, read from
> > --
> > 2.41.0.rc0.172.g3f132b7071-goog
> >

-- 

- Arnaldo

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

end of thread, other threads:[~2023-06-01 20:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-01  2:36 [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once Ian Rogers
2023-06-01  2:36 ` [PATCH v1 2/2] perf pmu: Warn about invalid config for all PMUs and configs Ian Rogers
2023-06-01 20:05   ` Namhyung Kim
2023-06-01 20:04 ` [PATCH v1 1/2] perf pmu: Only warn about unsupported configs once Namhyung Kim
2023-06-01 20:19   ` Arnaldo Carvalho de Melo

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).