1. Patchew
  2. linux
  3. [PATCH v1 0/5] NMI warning and debug improvements
  4. View patch

[PATCH v1 1/5] perf stat: Better hybrid support for the NMI watchdog warning

Ian Rogers posted 5 patches 9 months ago
There is a newer version of this series
[PATCH v1 1/5] perf stat: Better hybrid support for the NMI watchdog warning
Posted by Ian Rogers 9 months ago 
Prior to this patch evlist__has_hybrid would return false if the
processor wasn't hybrid or the evlist didn't contain any core
events. If the only PMU used by events was cpu_core then it would
true even though there are no cpu_atom events. For example:

```
$ perf stat --cputype=cpu_core -e '{cycles,cycles,cycles,cycles,cycles,cycles,cycles,cycles,cycles}' true

 Performance counter stats for 'true':

     <not counted>      cpu_core/cycles/                                                        (0.00%)
     <not counted>      cpu_core/cycles/                                                        (0.00%)
     <not counted>      cpu_core/cycles/                                                        (0.00%)
     <not counted>      cpu_core/cycles/                                                        (0.00%)
     <not counted>      cpu_core/cycles/                                                        (0.00%)
     <not counted>      cpu_core/cycles/                                                        (0.00%)
     <not counted>      cpu_core/cycles/                                                        (0.00%)
     <not counted>      cpu_core/cycles/                                                        (0.00%)
     <not counted>      cpu_core/cycles/                                                        (0.00%)

       0.001981900 seconds time elapsed

       0.002311000 seconds user
       0.000000000 seconds sys
```

This patch changes evlist__has_hybrid to return true only if the
evlist contains events from >1 core PMU. This means the NMI watchdog
warning is shown for the case above.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/stat-display.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index e852ac0d9847..f311f1960e29 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -825,13 +825,25 @@ static bool is_mixed_hw_group(struct evsel *counter)
 static bool evlist__has_hybrid(struct evlist *evlist)
 {
 	struct evsel *evsel;
+	struct perf_pmu *last_core_pmu = NULL;
 
 	if (perf_pmus__num_core_pmus() == 1)
 		return false;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->core.is_pmu_core)
+		if (evsel->core.is_pmu_core) {
+			struct perf_pmu *pmu = evsel__find_pmu(evsel);
+
+			if (pmu == last_core_pmu)
+				continue;
+
+			if (last_core_pmu == NULL) {
+				last_core_pmu = pmu;
+				continue;
+			}
+			/* A distinct core PMU. */
 			return true;
+		}
 	}
 
 	return false;
-- 
2.49.0.rc1.451.g8f38331e32-goog
Re: [PATCH v1 1/5] perf stat: Better hybrid support for the NMI watchdog warning
Posted by Liang, Kan 8 months, 2 weeks ago 

On 2025-03-18 12:14 a.m., Ian Rogers wrote:
> Prior to this patch evlist__has_hybrid would return false if the
> processor wasn't hybrid or the evlist didn't contain any core
> events. If the only PMU used by events was cpu_core then it would
> true even though there are no cpu_atom events. For example:
> 
> ```
> $ perf stat --cputype=cpu_core -e '{cycles,cycles,cycles,cycles,cycles,cycles,cycles,cycles,cycles}' true
> 
>  Performance counter stats for 'true':
> 
>      <not counted>      cpu_core/cycles/                                                        (0.00%)
>      <not counted>      cpu_core/cycles/                                                        (0.00%)
>      <not counted>      cpu_core/cycles/                                                        (0.00%)
>      <not counted>      cpu_core/cycles/                                                        (0.00%)
>      <not counted>      cpu_core/cycles/                                                        (0.00%)
>      <not counted>      cpu_core/cycles/                                                        (0.00%)
>      <not counted>      cpu_core/cycles/                                                        (0.00%)
>      <not counted>      cpu_core/cycles/                                                        (0.00%)
>      <not counted>      cpu_core/cycles/                                                        (0.00%)
> 
>        0.001981900 seconds time elapsed
> 
>        0.002311000 seconds user
>        0.000000000 seconds sys
> ```
> 
> This patch changes evlist__has_hybrid to return true only if the
> evlist contains events from >1 core PMU. This means the NMI watchdog
> warning is shown for the case above.

Nit:
The function name may still bring confusions.
It may be better to change the function name as well, e.g.,
evlist__has_hybrid_pmus()? It implies more than one PMU.

Thanks,
Kan>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/stat-display.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index e852ac0d9847..f311f1960e29 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -825,13 +825,25 @@ static bool is_mixed_hw_group(struct evsel *counter)
>  static bool evlist__has_hybrid(struct evlist *evlist)
>  {
>  	struct evsel *evsel;
> +	struct perf_pmu *last_core_pmu = NULL;
>  
>  	if (perf_pmus__num_core_pmus() == 1)
>  		return false;
>  
>  	evlist__for_each_entry(evlist, evsel) {
> -		if (evsel->core.is_pmu_core)
> +		if (evsel->core.is_pmu_core) {
> +			struct perf_pmu *pmu = evsel__find_pmu(evsel);
> +
> +			if (pmu == last_core_pmu)
> +				continue;
> +
> +			if (last_core_pmu == NULL) {
> +				last_core_pmu = pmu;
> +				continue;
> +			}
> +			/* A distinct core PMU. */
>  			return true;
> +		}
>  	}
>  
>  	return false;
Re: [PATCH v1 1/5] perf stat: Better hybrid support for the NMI watchdog warning
Posted by Ian Rogers 8 months, 2 weeks ago 
On Wed, Apr 2, 2025 at 8:23 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2025-03-18 12:14 a.m., Ian Rogers wrote:
> > Prior to this patch evlist__has_hybrid would return false if the
> > processor wasn't hybrid or the evlist didn't contain any core
> > events. If the only PMU used by events was cpu_core then it would
> > true even though there are no cpu_atom events. For example:
> >
> > ```
> > $ perf stat --cputype=cpu_core -e '{cycles,cycles,cycles,cycles,cycles,cycles,cycles,cycles,cycles}' true
> >
> >  Performance counter stats for 'true':
> >
> >      <not counted>      cpu_core/cycles/                                                        (0.00%)
> >      <not counted>      cpu_core/cycles/                                                        (0.00%)
> >      <not counted>      cpu_core/cycles/                                                        (0.00%)
> >      <not counted>      cpu_core/cycles/                                                        (0.00%)
> >      <not counted>      cpu_core/cycles/                                                        (0.00%)
> >      <not counted>      cpu_core/cycles/                                                        (0.00%)
> >      <not counted>      cpu_core/cycles/                                                        (0.00%)
> >      <not counted>      cpu_core/cycles/                                                        (0.00%)
> >      <not counted>      cpu_core/cycles/                                                        (0.00%)
> >
> >        0.001981900 seconds time elapsed
> >
> >        0.002311000 seconds user
> >        0.000000000 seconds sys
> > ```
> >
> > This patch changes evlist__has_hybrid to return true only if the
> > evlist contains events from >1 core PMU. This means the NMI watchdog
> > warning is shown for the case above.
>
> Nit:
> The function name may still bring confusions.
> It may be better to change the function name as well, e.g.,
> evlist__has_hybrid_pmus()? It implies more than one PMU.

Thanks, I'll change in v2.

Ian

> Thanks,
> Kan>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/util/stat-display.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index e852ac0d9847..f311f1960e29 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -825,13 +825,25 @@ static bool is_mixed_hw_group(struct evsel *counter)
> >  static bool evlist__has_hybrid(struct evlist *evlist)
> >  {
> >       struct evsel *evsel;
> > +     struct perf_pmu *last_core_pmu = NULL;
> >
> >       if (perf_pmus__num_core_pmus() == 1)
> >               return false;
> >
> >       evlist__for_each_entry(evlist, evsel) {
> > -             if (evsel->core.is_pmu_core)
> > +             if (evsel->core.is_pmu_core) {
> > +                     struct perf_pmu *pmu = evsel__find_pmu(evsel);
> > +
> > +                     if (pmu == last_core_pmu)
> > +                             continue;
> > +
> > +                     if (last_core_pmu == NULL) {
> > +                             last_core_pmu = pmu;
> > +                             continue;
> > +                     }
> > +                     /* A distinct core PMU. */
> >                       return true;
> > +             }
> >       }
> >
> >       return false;
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.