linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
@ 2022-09-22  7:10 Dongli Zhang
  2022-09-22  8:07 ` Like Xu
  2022-10-14 22:16 ` Namhyung Kim
  0 siblings, 2 replies; 10+ messages in thread
From: Dongli Zhang @ 2022-09-22  7:10 UTC (permalink / raw)
  To: linux-perf-users, kvm
  Cc: linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung, joe.jin, likexu

Add kvm@vger.kernel.org as this issue is in virtualization env.

The topdown metrics events became default since
commit 42641d6f4d15 ("perf stat: Add Topdown metrics events as default
events"). The perf will use 'slots' if the
/sys/bus/event_source/devices/cpu/events/slots is available.

Unfortunately, the 'slots' may not be supported in the virualization
environment. The hypervisor may not expose the 'slots' counter to the VM
in cpuid. As a result, the kernel may disable topdown slots and metrics
events in intel_pmu_init() if slots event is not in CPUID. E.g., both
c->weight and c->idxmsk64 are set to 0.

There will be below error on Icelake VM since 'slots' is the leader:

$ perf stat
Error:
The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (slots).
/bin/dmesg | grep -i perf may provide additional information.

This is because the stat_handle_error() returns COUNTER_FATAL when the
'slots' is used as leader of events.

There are three options to fix the issue.

1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
userspace so that pmu_have_event(pmu->name, "slots") returns false.

2. Run cpuid at perf userspace and avoid using 'slots' if it is not
supported in cpuid.

3. Do not fatal perf if the leader is failed. Do not create events for an
evsel if its leader is already failed.

This RFC patch is with the 3rd option. Would you mind suggesting which
option is better?

Here is the output of patch.

$ perf stat -v
Using CPUID GenuineIntel-6-6A-6
slots -> cpu/event=0,umask=0x4/
topdown-retiring -> cpu/event=0,umask=0x80/
topdown-bad-spec -> cpu/event=0,umask=0x81/
topdown-fe-bound -> cpu/event=0,umask=0x82/
topdown-be-bound -> cpu/event=0,umask=0x83/
Control descriptor is not initialized
Warning:
slots event is not supported by the kernel.
^Ccpu-clock: 62021481051 62021480237 62021480237
context-switches: 437 62021478064 62021478064
cpu-migrations: 17 62021475294 62021475294
page-faults: 12 62021471925 62021471925
cycles: 15662273 62020909141 62020909141
instructions: 6580385 62008944246 62008944246
branches: 1446119 62008855550 62008855550
branch-misses: 30970 62008643255 62008643255
failed to read counter slots
failed to read counter topdown-retiring
failed to read counter topdown-bad-spec
failed to read counter topdown-fe-bound
failed to read counter topdown-be-bound

 Performance counter stats for 'system wide':

         62,021.48 msec cpu-clock                        #   16.006 CPUs utilized          
               437      context-switches                 #    7.046 /sec                   
                17      cpu-migrations                   #    0.274 /sec                   
                12      page-faults                      #    0.193 /sec                   
        15,662,273      cycles                           #    0.000 GHz                    
         6,580,385      instructions                     #    0.42  insn per cycle         
         1,446,119      branches                         #   23.316 K/sec                  
            30,970      branch-misses                    #    2.14% of all branches        
   <not supported>      slots                                                       
   <not supported>      topdown-retiring                                            
   <not supported>      topdown-bad-spec                                            
   <not supported>      topdown-fe-bound                                            
   <not supported>      topdown-be-bound                                            

       3.874991326 seconds time elapsed

Thank you very much!

Cc: Joe Jin <joe.jin@oracle.com>
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
 tools/perf/builtin-stat.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 0b4a62e4ff67..1053cf0886c0 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -762,9 +762,7 @@ static enum counter_recovery stat_handle_error(struct evsel *counter)
 		 */
 		counter->errored = true;
 
-		if ((evsel__leader(counter) != counter) ||
-		    !(counter->core.leader->nr_members > 1))
-			return COUNTER_SKIP;
+		return COUNTER_SKIP;
 	} else if (evsel__fallback(counter, errno, msg, sizeof(msg))) {
 		if (verbose > 0)
 			ui__warning("%s\n", msg);
@@ -843,6 +841,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 		if (target.use_bpf)
 			break;
 
+		if (evsel__leader(counter)->errored)
+			continue;
 		if (counter->reset_group || counter->errored)
 			continue;
 		if (evsel__is_bpf(counter))
@@ -901,6 +901,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 		evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
 			counter = evlist_cpu_itr.evsel;
 
+			if (evsel__leader(counter)->errored)
+				continue;
 			if (!counter->reset_group && !counter->errored)
 				continue;
 			if (!counter->reset_group)
-- 
2.17.1


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

* Re: [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
  2022-09-22  7:10 [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored Dongli Zhang
@ 2022-09-22  8:07 ` Like Xu
  2022-09-22 13:34   ` Liang, Kan
  2022-09-22 18:10   ` Dongli Zhang
  2022-10-14 22:16 ` Namhyung Kim
  1 sibling, 2 replies; 10+ messages in thread
From: Like Xu @ 2022-09-22  8:07 UTC (permalink / raw)
  To: Dongli Zhang
  Cc: linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung, joe.jin, linux-perf-users,
	kvm list, Kan Liang

On 22/9/2022 3:10 pm, Dongli Zhang wrote:
> There are three options to fix the issue.
> 
> 1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
> userspace so that pmu_have_event(pmu->name, "slots") returns false.

IMO, the guest PMU driver should be fixed
since it misrepresents emulated hardware capabilities in terms of slots.

> 
> 2. Run cpuid at perf userspace and avoid using 'slots' if it is not
> supported in cpuid.
> 
> 3. Do not fatal perf if the leader is failed. Do not create events for an
> evsel if its leader is already failed.

We may also need this since it's easier and more agile to update the perf tool
than the kernel code or KVM emulated capabilities.

> 
> This RFC patch is with the 3rd option. Would you mind suggesting which
> option is better?

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

* Re: [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
  2022-09-22  8:07 ` Like Xu
@ 2022-09-22 13:34   ` Liang, Kan
  2022-09-22 18:00     ` Dongli Zhang
  2022-09-22 18:10   ` Dongli Zhang
  1 sibling, 1 reply; 10+ messages in thread
From: Liang, Kan @ 2022-09-22 13:34 UTC (permalink / raw)
  To: Like Xu, Dongli Zhang
  Cc: linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung, joe.jin, linux-perf-users,
	kvm list



On 2022-09-22 4:07 a.m., Like Xu wrote:
> On 22/9/2022 3:10 pm, Dongli Zhang wrote:
>> There are three options to fix the issue.
>>
>> 1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
>> userspace so that pmu_have_event(pmu->name, "slots") returns false.
> 
> IMO, the guest PMU driver should be fixed
> since it misrepresents emulated hardware capabilities in terms of slots.

Yes, we need to fix the kernel to hide the slots event if it's not
available.

The patch as below should fix it. (Not tested yet)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index b20e646c8205..27ee43faba32 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -5565,6 +5565,19 @@ static struct attribute *intel_pmu_attrs[] = {
 	NULL,
 };

+static umode_t
+td_is_visible(struct kobject *kobj, struct attribute *attr, int i)
+{
+	/*
+	 * Hide the perf metrics topdown events
+	 * if the slots is not in CPUID.
+	 */
+	if (x86_pmu.num_topdown_events)
+		return (x86_pmu.intel_ctrl & INTEL_PMC_MSK_FIXED_SLOTS) ? attr->mode : 0;
+
+	return attr->mode;
+}
+
 static umode_t
 tsx_is_visible(struct kobject *kobj, struct attribute *attr, int i)
 {
@@ -5600,6 +5613,7 @@ default_is_visible(struct kobject *kobj, struct
attribute *attr, int i)

 static struct attribute_group group_events_td  = {
 	.name = "events",
+	.is_visible = td_is_visible,
 };

 static struct attribute_group group_events_mem = {
@@ -5758,6 +5772,23 @@ static inline int
hybrid_find_supported_cpu(struct x86_hybrid_pmu *pmu)
 	return (cpu >= nr_cpu_ids) ? -1 : cpu;
 }

+static umode_t hybrid_td_is_visible(struct kobject *kobj,
+					struct attribute *attr, int i)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct x86_hybrid_pmu *pmu =
+		 container_of(dev_get_drvdata(dev), struct x86_hybrid_pmu, pmu);
+
+	if (!is_attr_for_this_pmu(kobj, attr))
+		return 0;
+
+	/* Only check the big core which supports perf metrics */
+	if (pmu->cpu_type == hybrid_big)
+		return (pmu->intel_ctrl & INTEL_PMC_MSK_FIXED_SLOTS) ? attr->mode : 0;
+
+	return attr->mode;
+}
+
 static umode_t hybrid_tsx_is_visible(struct kobject *kobj,
 				     struct attribute *attr, int i)
 {
@@ -5784,7 +5815,7 @@ static umode_t hybrid_format_is_visible(struct
kobject *kobj,

 static struct attribute_group hybrid_group_events_td  = {
 	.name		= "events",
-	.is_visible	= hybrid_events_is_visible,
+	.is_visible	= hybrid_td_is_visible,
 };

 static struct attribute_group hybrid_group_events_mem = {


Thanks,
Kan

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

* Re: [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
  2022-09-22 13:34   ` Liang, Kan
@ 2022-09-22 18:00     ` Dongli Zhang
  2022-09-22 18:42       ` Liang, Kan
  0 siblings, 1 reply; 10+ messages in thread
From: Dongli Zhang @ 2022-09-22 18:00 UTC (permalink / raw)
  To: Liang, Kan, Like Xu
  Cc: linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung, joe.jin, linux-perf-users,
	kvm list

Hi Kan,

I have tested that the patch works by hiding 'slots' sysfs entries.

# ll /sys/bus/event_source/devices/cpu/events/
total 0
-r--r--r--. 1 root root 4096 Sep 22 17:57 branch-instructions
-r--r--r--. 1 root root 4096 Sep 22 17:57 branch-misses
-r--r--r--. 1 root root 4096 Sep 22 17:57 bus-cycles
-r--r--r--. 1 root root 4096 Sep 22 17:57 cache-misses
-r--r--r--. 1 root root 4096 Sep 22 17:57 cache-references
-r--r--r--. 1 root root 4096 Sep 22 17:57 cpu-cycles
-r--r--r--. 1 root root 4096 Sep 22 17:57 instructions
-r--r--r--. 1 root root 4096 Sep 22 17:57 ref-cycles

# perf stat
^C
 Performance counter stats for 'system wide':

         19,256.20 msec cpu-clock                        #   16.025 CPUs
utilized
               179      context-switches                 #    9.296 /sec

                17      cpu-migrations                   #    0.883 /sec

                 3      page-faults                      #    0.156 /sec

         7,502,294      cycles                           #    0.000 GHz

         2,512,587      instructions                     #    0.33  insn per
cycle
           552,989      branches                         #   28.717 K/sec

            15,999      branch-misses                    #    2.89% of all
branches

       1.201628129 seconds time elapsed


Would you send this patch to fix at the kernel side?

Thank you very much!

Dongli Zhang

On 9/22/22 6:34 AM, Liang, Kan wrote:
> 
> 
> On 2022-09-22 4:07 a.m., Like Xu wrote:
>> On 22/9/2022 3:10 pm, Dongli Zhang wrote:
>>> There are three options to fix the issue.
>>>
>>> 1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
>>> userspace so that pmu_have_event(pmu->name, "slots") returns false.
>>
>> IMO, the guest PMU driver should be fixed
>> since it misrepresents emulated hardware capabilities in terms of slots.
> 
> Yes, we need to fix the kernel to hide the slots event if it's not
> available.
> 
> The patch as below should fix it. (Not tested yet)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index b20e646c8205..27ee43faba32 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -5565,6 +5565,19 @@ static struct attribute *intel_pmu_attrs[] = {
>  	NULL,
>  };
> 
> +static umode_t
> +td_is_visible(struct kobject *kobj, struct attribute *attr, int i)
> +{
> +	/*
> +	 * Hide the perf metrics topdown events
> +	 * if the slots is not in CPUID.
> +	 */
> +	if (x86_pmu.num_topdown_events)
> +		return (x86_pmu.intel_ctrl & INTEL_PMC_MSK_FIXED_SLOTS) ? attr->mode : 0;
> +
> +	return attr->mode;
> +}
> +
>  static umode_t
>  tsx_is_visible(struct kobject *kobj, struct attribute *attr, int i)
>  {
> @@ -5600,6 +5613,7 @@ default_is_visible(struct kobject *kobj, struct
> attribute *attr, int i)
> 
>  static struct attribute_group group_events_td  = {
>  	.name = "events",
> +	.is_visible = td_is_visible,
>  };
> 
>  static struct attribute_group group_events_mem = {
> @@ -5758,6 +5772,23 @@ static inline int
> hybrid_find_supported_cpu(struct x86_hybrid_pmu *pmu)
>  	return (cpu >= nr_cpu_ids) ? -1 : cpu;
>  }
> 
> +static umode_t hybrid_td_is_visible(struct kobject *kobj,
> +					struct attribute *attr, int i)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct x86_hybrid_pmu *pmu =
> +		 container_of(dev_get_drvdata(dev), struct x86_hybrid_pmu, pmu);
> +
> +	if (!is_attr_for_this_pmu(kobj, attr))
> +		return 0;
> +
> +	/* Only check the big core which supports perf metrics */
> +	if (pmu->cpu_type == hybrid_big)
> +		return (pmu->intel_ctrl & INTEL_PMC_MSK_FIXED_SLOTS) ? attr->mode : 0;
> +
> +	return attr->mode;
> +}
> +
>  static umode_t hybrid_tsx_is_visible(struct kobject *kobj,
>  				     struct attribute *attr, int i)
>  {
> @@ -5784,7 +5815,7 @@ static umode_t hybrid_format_is_visible(struct
> kobject *kobj,
> 
>  static struct attribute_group hybrid_group_events_td  = {
>  	.name		= "events",
> -	.is_visible	= hybrid_events_is_visible,
> +	.is_visible	= hybrid_td_is_visible,
>  };
> 
>  static struct attribute_group hybrid_group_events_mem = {
> 
> 
> Thanks,
> Kan
> 

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

* Re: [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
  2022-09-22  8:07 ` Like Xu
  2022-09-22 13:34   ` Liang, Kan
@ 2022-09-22 18:10   ` Dongli Zhang
  1 sibling, 0 replies; 10+ messages in thread
From: Dongli Zhang @ 2022-09-22 18:10 UTC (permalink / raw)
  To: Like Xu
  Cc: linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung, joe.jin, linux-perf-users,
	kvm list, Kan Liang, Bert Barbe



On 9/22/22 1:07 AM, Like Xu wrote:
> On 22/9/2022 3:10 pm, Dongli Zhang wrote:
>> There are three options to fix the issue.
>>
>> 1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
>> userspace so that pmu_have_event(pmu->name, "slots") returns false.
> 
> IMO, the guest PMU driver should be fixed
> since it misrepresents emulated hardware capabilities in terms of slots.
> 
>>
>> 2. Run cpuid at perf userspace and avoid using 'slots' if it is not
>> supported in cpuid.
>>
>> 3. Do not fatal perf if the leader is failed. Do not create events for an
>> evsel if its leader is already failed.
> 
> We may also need this since it's easier and more agile to update the perf tool
> than the kernel code or KVM emulated capabilities.

Thank you very much for the suggestion. I may remove the RFC and re-send the
patch for perf userspace.

BTW, I used to try your prior patch at KVM side (suggested by Bert) but it does
not help in this case. The VM kernel is already aware that 'slots' is not
supported by the hypervisor.

https://lore.kernel.org/lkml/20220106032118.34459-1-likexu@tencent.com/T/

Dongli Zhang

> 
>>
>> This RFC patch is with the 3rd option. Would you mind suggesting which
>> option is better?

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

* Re: [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
  2022-09-22 18:00     ` Dongli Zhang
@ 2022-09-22 18:42       ` Liang, Kan
  0 siblings, 0 replies; 10+ messages in thread
From: Liang, Kan @ 2022-09-22 18:42 UTC (permalink / raw)
  To: Dongli Zhang, Like Xu
  Cc: linux-kernel, peterz, mingo, acme, mark.rutland,
	alexander.shishkin, jolsa, namhyung, joe.jin, linux-perf-users,
	kvm list



On 2022-09-22 2:00 p.m., Dongli Zhang wrote:
> Hi Kan,
> 
> I have tested that the patch works by hiding 'slots' sysfs entries.
> 
> # ll /sys/bus/event_source/devices/cpu/events/
> total 0
> -r--r--r--. 1 root root 4096 Sep 22 17:57 branch-instructions
> -r--r--r--. 1 root root 4096 Sep 22 17:57 branch-misses
> -r--r--r--. 1 root root 4096 Sep 22 17:57 bus-cycles
> -r--r--r--. 1 root root 4096 Sep 22 17:57 cache-misses
> -r--r--r--. 1 root root 4096 Sep 22 17:57 cache-references
> -r--r--r--. 1 root root 4096 Sep 22 17:57 cpu-cycles
> -r--r--r--. 1 root root 4096 Sep 22 17:57 instructions
> -r--r--r--. 1 root root 4096 Sep 22 17:57 ref-cycles
> 
> # perf stat
> ^C
>  Performance counter stats for 'system wide':
> 
>          19,256.20 msec cpu-clock                        #   16.025 CPUs
> utilized
>                179      context-switches                 #    9.296 /sec
> 
>                 17      cpu-migrations                   #    0.883 /sec
> 
>                  3      page-faults                      #    0.156 /sec
> 
>          7,502,294      cycles                           #    0.000 GHz
> 
>          2,512,587      instructions                     #    0.33  insn per
> cycle
>            552,989      branches                         #   28.717 K/sec
> 
>             15,999      branch-misses                    #    2.89% of all
> branches
> 
>        1.201628129 seconds time elapsed
> 
> 
> Would you send this patch to fix at the kernel side?

Yes, I will send it out shortly.


Thanks,
Kan
> 
> Thank you very much!
> 
> Dongli Zhang
> 
> On 9/22/22 6:34 AM, Liang, Kan wrote:
>>
>>
>> On 2022-09-22 4:07 a.m., Like Xu wrote:
>>> On 22/9/2022 3:10 pm, Dongli Zhang wrote:
>>>> There are three options to fix the issue.
>>>>
>>>> 1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
>>>> userspace so that pmu_have_event(pmu->name, "slots") returns false.
>>>
>>> IMO, the guest PMU driver should be fixed
>>> since it misrepresents emulated hardware capabilities in terms of slots.
>>
>> Yes, we need to fix the kernel to hide the slots event if it's not
>> available.
>>
>> The patch as below should fix it. (Not tested yet)
>>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index b20e646c8205..27ee43faba32 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -5565,6 +5565,19 @@ static struct attribute *intel_pmu_attrs[] = {
>>  	NULL,
>>  };
>>
>> +static umode_t
>> +td_is_visible(struct kobject *kobj, struct attribute *attr, int i)
>> +{
>> +	/*
>> +	 * Hide the perf metrics topdown events
>> +	 * if the slots is not in CPUID.
>> +	 */
>> +	if (x86_pmu.num_topdown_events)
>> +		return (x86_pmu.intel_ctrl & INTEL_PMC_MSK_FIXED_SLOTS) ? attr->mode : 0;
>> +
>> +	return attr->mode;
>> +}
>> +
>>  static umode_t
>>  tsx_is_visible(struct kobject *kobj, struct attribute *attr, int i)
>>  {
>> @@ -5600,6 +5613,7 @@ default_is_visible(struct kobject *kobj, struct
>> attribute *attr, int i)
>>
>>  static struct attribute_group group_events_td  = {
>>  	.name = "events",
>> +	.is_visible = td_is_visible,
>>  };
>>
>>  static struct attribute_group group_events_mem = {
>> @@ -5758,6 +5772,23 @@ static inline int
>> hybrid_find_supported_cpu(struct x86_hybrid_pmu *pmu)
>>  	return (cpu >= nr_cpu_ids) ? -1 : cpu;
>>  }
>>
>> +static umode_t hybrid_td_is_visible(struct kobject *kobj,
>> +					struct attribute *attr, int i)
>> +{
>> +	struct device *dev = kobj_to_dev(kobj);
>> +	struct x86_hybrid_pmu *pmu =
>> +		 container_of(dev_get_drvdata(dev), struct x86_hybrid_pmu, pmu);
>> +
>> +	if (!is_attr_for_this_pmu(kobj, attr))
>> +		return 0;
>> +
>> +	/* Only check the big core which supports perf metrics */
>> +	if (pmu->cpu_type == hybrid_big)
>> +		return (pmu->intel_ctrl & INTEL_PMC_MSK_FIXED_SLOTS) ? attr->mode : 0;
>> +
>> +	return attr->mode;
>> +}
>> +
>>  static umode_t hybrid_tsx_is_visible(struct kobject *kobj,
>>  				     struct attribute *attr, int i)
>>  {
>> @@ -5784,7 +5815,7 @@ static umode_t hybrid_format_is_visible(struct
>> kobject *kobj,
>>
>>  static struct attribute_group hybrid_group_events_td  = {
>>  	.name		= "events",
>> -	.is_visible	= hybrid_events_is_visible,
>> +	.is_visible	= hybrid_td_is_visible,
>>  };
>>
>>  static struct attribute_group hybrid_group_events_mem = {
>>
>>
>> Thanks,
>> Kan
>>

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

* Re: [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
  2022-09-22  7:10 [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored Dongli Zhang
  2022-09-22  8:07 ` Like Xu
@ 2022-10-14 22:16 ` Namhyung Kim
  2022-10-14 23:47   ` Dongli Zhang
  2022-10-18 21:29   ` Dongli Zhang
  1 sibling, 2 replies; 10+ messages in thread
From: Namhyung Kim @ 2022-10-14 22:16 UTC (permalink / raw)
  To: Dongli Zhang
  Cc: linux-perf-users, KVM, linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, joe.jin, Like Xu

Hello,

On Thu, Sep 22, 2022 at 12:10 AM Dongli Zhang <dongli.zhang@oracle.com> wrote:
>
> Add kvm@vger.kernel.org as this issue is in virtualization env.
>
> The topdown metrics events became default since
> commit 42641d6f4d15 ("perf stat: Add Topdown metrics events as default
> events"). The perf will use 'slots' if the
> /sys/bus/event_source/devices/cpu/events/slots is available.
>
> Unfortunately, the 'slots' may not be supported in the virualization
> environment. The hypervisor may not expose the 'slots' counter to the VM
> in cpuid. As a result, the kernel may disable topdown slots and metrics
> events in intel_pmu_init() if slots event is not in CPUID. E.g., both
> c->weight and c->idxmsk64 are set to 0.
>
> There will be below error on Icelake VM since 'slots' is the leader:
>
> $ perf stat
> Error:
> The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (slots).
> /bin/dmesg | grep -i perf may provide additional information.
>
> This is because the stat_handle_error() returns COUNTER_FATAL when the
> 'slots' is used as leader of events.
>
> There are three options to fix the issue.
>
> 1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
> userspace so that pmu_have_event(pmu->name, "slots") returns false.
>
> 2. Run cpuid at perf userspace and avoid using 'slots' if it is not
> supported in cpuid.
>
> 3. Do not fatal perf if the leader is failed. Do not create events for an
> evsel if its leader is already failed.
>
> This RFC patch is with the 3rd option. Would you mind suggesting which
> option is better?

Sorry for the late reply but I think option 1 is the way to go.

The option 3 would be a transient workaround and it would affect
other events too.  If it's really needed, I think option 2 is slightly better
than option 3.  Or, we can add --force option to skip non-supported
events explicitly.

Thanks,
Namhyung

>
> Here is the output of patch.
>
> $ perf stat -v
> Using CPUID GenuineIntel-6-6A-6
> slots -> cpu/event=0,umask=0x4/
> topdown-retiring -> cpu/event=0,umask=0x80/
> topdown-bad-spec -> cpu/event=0,umask=0x81/
> topdown-fe-bound -> cpu/event=0,umask=0x82/
> topdown-be-bound -> cpu/event=0,umask=0x83/
> Control descriptor is not initialized
> Warning:
> slots event is not supported by the kernel.
> ^Ccpu-clock: 62021481051 62021480237 62021480237
> context-switches: 437 62021478064 62021478064
> cpu-migrations: 17 62021475294 62021475294
> page-faults: 12 62021471925 62021471925
> cycles: 15662273 62020909141 62020909141
> instructions: 6580385 62008944246 62008944246
> branches: 1446119 62008855550 62008855550
> branch-misses: 30970 62008643255 62008643255
> failed to read counter slots
> failed to read counter topdown-retiring
> failed to read counter topdown-bad-spec
> failed to read counter topdown-fe-bound
> failed to read counter topdown-be-bound
>
>  Performance counter stats for 'system wide':
>
>          62,021.48 msec cpu-clock                        #   16.006 CPUs utilized
>                437      context-switches                 #    7.046 /sec
>                 17      cpu-migrations                   #    0.274 /sec
>                 12      page-faults                      #    0.193 /sec
>         15,662,273      cycles                           #    0.000 GHz
>          6,580,385      instructions                     #    0.42  insn per cycle
>          1,446,119      branches                         #   23.316 K/sec
>             30,970      branch-misses                    #    2.14% of all branches
>    <not supported>      slots
>    <not supported>      topdown-retiring
>    <not supported>      topdown-bad-spec
>    <not supported>      topdown-fe-bound
>    <not supported>      topdown-be-bound
>
>        3.874991326 seconds time elapsed
>
> Thank you very much!
>
> Cc: Joe Jin <joe.jin@oracle.com>
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> ---
>  tools/perf/builtin-stat.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 0b4a62e4ff67..1053cf0886c0 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -762,9 +762,7 @@ static enum counter_recovery stat_handle_error(struct evsel *counter)
>                  */
>                 counter->errored = true;
>
> -               if ((evsel__leader(counter) != counter) ||
> -                   !(counter->core.leader->nr_members > 1))
> -                       return COUNTER_SKIP;
> +               return COUNTER_SKIP;
>         } else if (evsel__fallback(counter, errno, msg, sizeof(msg))) {
>                 if (verbose > 0)
>                         ui__warning("%s\n", msg);
> @@ -843,6 +841,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>                 if (target.use_bpf)
>                         break;
>
> +               if (evsel__leader(counter)->errored)
> +                       continue;
>                 if (counter->reset_group || counter->errored)
>                         continue;
>                 if (evsel__is_bpf(counter))
> @@ -901,6 +901,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>                 evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
>                         counter = evlist_cpu_itr.evsel;
>
> +                       if (evsel__leader(counter)->errored)
> +                               continue;
>                         if (!counter->reset_group && !counter->errored)
>                                 continue;
>                         if (!counter->reset_group)
> --
> 2.17.1
>

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

* Re: [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
  2022-10-14 22:16 ` Namhyung Kim
@ 2022-10-14 23:47   ` Dongli Zhang
  2022-10-18 21:29   ` Dongli Zhang
  1 sibling, 0 replies; 10+ messages in thread
From: Dongli Zhang @ 2022-10-14 23:47 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-perf-users, KVM, linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, joe.jin, Like Xu, kan.liang

Hi Namhyung,

On 10/14/22 3:16 PM, Namhyung Kim wrote:
> Hello,
> 
> On Thu, Sep 22, 2022 at 12:10 AM Dongli Zhang <dongli.zhang@oracle.com> wrote:
>>
>> Add kvm@vger.kernel.org as this issue is in virtualization env.
>>
>> The topdown metrics events became default since
>> commit 42641d6f4d15 ("perf stat: Add Topdown metrics events as default
>> events"). The perf will use 'slots' if the
>> /sys/bus/event_source/devices/cpu/events/slots is available.
>>
>> Unfortunately, the 'slots' may not be supported in the virualization
>> environment. The hypervisor may not expose the 'slots' counter to the VM
>> in cpuid. As a result, the kernel may disable topdown slots and metrics
>> events in intel_pmu_init() if slots event is not in CPUID. E.g., both
>> c->weight and c->idxmsk64 are set to 0.
>>
>> There will be below error on Icelake VM since 'slots' is the leader:
>>
>> $ perf stat
>> Error:
>> The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (slots).
>> /bin/dmesg | grep -i perf may provide additional information.
>>
>> This is because the stat_handle_error() returns COUNTER_FATAL when the
>> 'slots' is used as leader of events.
>>
>> There are three options to fix the issue.
>>
>> 1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
>> userspace so that pmu_have_event(pmu->name, "slots") returns false.
>>
>> 2. Run cpuid at perf userspace and avoid using 'slots' if it is not
>> supported in cpuid.
>>
>> 3. Do not fatal perf if the leader is failed. Do not create events for an
>> evsel if its leader is already failed.
>>
>> This RFC patch is with the 3rd option. Would you mind suggesting which
>> option is better?
> 
> Sorry for the late reply but I think option 1 is the way to go.
> 
> The option 3 would be a transient workaround and it would affect
> other events too.  If it's really needed, I think option 2 is slightly better
> than option 3.  Or, we can add --force option to skip non-supported
> events explicitly.

About option 2 and 3, I will wait for more comments if anyone still prefers the
change at userspace, e.g., the "--force" option to avoid failure but just to
report non-supported.

I have already sent a version that may impact other events. Please ignore as you
think option 1 is enough.

https://lore.kernel.org/all/20221010050113.13050-1-dongli.zhang@oracle.com/


About option 1, there is a bugfix from Ken pending for review.

https://lore.kernel.org/all/20220922201505.2721654-1-kan.liang@linux.intel.com/

Thank you very much!

Dongli Zhang

> 
> Thanks,
> Namhyung
> 
>>
>> Here is the output of patch.
>>
>> $ perf stat -v
>> Using CPUID GenuineIntel-6-6A-6
>> slots -> cpu/event=0,umask=0x4/
>> topdown-retiring -> cpu/event=0,umask=0x80/
>> topdown-bad-spec -> cpu/event=0,umask=0x81/
>> topdown-fe-bound -> cpu/event=0,umask=0x82/
>> topdown-be-bound -> cpu/event=0,umask=0x83/
>> Control descriptor is not initialized
>> Warning:
>> slots event is not supported by the kernel.
>> ^Ccpu-clock: 62021481051 62021480237 62021480237
>> context-switches: 437 62021478064 62021478064
>> cpu-migrations: 17 62021475294 62021475294
>> page-faults: 12 62021471925 62021471925
>> cycles: 15662273 62020909141 62020909141
>> instructions: 6580385 62008944246 62008944246
>> branches: 1446119 62008855550 62008855550
>> branch-misses: 30970 62008643255 62008643255
>> failed to read counter slots
>> failed to read counter topdown-retiring
>> failed to read counter topdown-bad-spec
>> failed to read counter topdown-fe-bound
>> failed to read counter topdown-be-bound
>>
>>  Performance counter stats for 'system wide':
>>
>>          62,021.48 msec cpu-clock                        #   16.006 CPUs utilized
>>                437      context-switches                 #    7.046 /sec
>>                 17      cpu-migrations                   #    0.274 /sec
>>                 12      page-faults                      #    0.193 /sec
>>         15,662,273      cycles                           #    0.000 GHz
>>          6,580,385      instructions                     #    0.42  insn per cycle
>>          1,446,119      branches                         #   23.316 K/sec
>>             30,970      branch-misses                    #    2.14% of all branches
>>    <not supported>      slots
>>    <not supported>      topdown-retiring
>>    <not supported>      topdown-bad-spec
>>    <not supported>      topdown-fe-bound
>>    <not supported>      topdown-be-bound
>>
>>        3.874991326 seconds time elapsed
>>
>> Thank you very much!
>>
>> Cc: Joe Jin <joe.jin@oracle.com>
>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>> ---
>>  tools/perf/builtin-stat.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 0b4a62e4ff67..1053cf0886c0 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -762,9 +762,7 @@ static enum counter_recovery stat_handle_error(struct evsel *counter)
>>                  */
>>                 counter->errored = true;
>>
>> -               if ((evsel__leader(counter) != counter) ||
>> -                   !(counter->core.leader->nr_members > 1))
>> -                       return COUNTER_SKIP;
>> +               return COUNTER_SKIP;
>>         } else if (evsel__fallback(counter, errno, msg, sizeof(msg))) {
>>                 if (verbose > 0)
>>                         ui__warning("%s\n", msg);
>> @@ -843,6 +841,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>>                 if (target.use_bpf)
>>                         break;
>>
>> +               if (evsel__leader(counter)->errored)
>> +                       continue;
>>                 if (counter->reset_group || counter->errored)
>>                         continue;
>>                 if (evsel__is_bpf(counter))
>> @@ -901,6 +901,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>>                 evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
>>                         counter = evlist_cpu_itr.evsel;
>>
>> +                       if (evsel__leader(counter)->errored)
>> +                               continue;
>>                         if (!counter->reset_group && !counter->errored)
>>                                 continue;
>>                         if (!counter->reset_group)
>> --
>> 2.17.1
>>

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

* Re: [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
  2022-10-14 22:16 ` Namhyung Kim
  2022-10-14 23:47   ` Dongli Zhang
@ 2022-10-18 21:29   ` Dongli Zhang
  2022-10-18 21:31     ` Dongli Zhang
  1 sibling, 1 reply; 10+ messages in thread
From: Dongli Zhang @ 2022-10-18 21:29 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-perf-users, KVM, linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, joe.jin, Like Xu



On 10/14/22 3:16 PM, Namhyung Kim wrote:
> Hello,
> 
> On Thu, Sep 22, 2022 at 12:10 AM Dongli Zhang <dongli.zhang@oracle.com> wrote:
>>
>> Add kvm@vger.kernel.org as this issue is in virtualization env.
>>
>> The topdown metrics events became default since
>> commit 42641d6f4d15 ("perf stat: Add Topdown metrics events as default
>> events"). The perf will use 'slots' if the
>> /sys/bus/event_source/devices/cpu/events/slots is available.
>>
>> Unfortunately, the 'slots' may not be supported in the virualization
>> environment. The hypervisor may not expose the 'slots' counter to the VM
>> in cpuid. As a result, the kernel may disable topdown slots and metrics
>> events in intel_pmu_init() if slots event is not in CPUID. E.g., both
>> c->weight and c->idxmsk64 are set to 0.
>>
>> There will be below error on Icelake VM since 'slots' is the leader:
>>
>> $ perf stat
>> Error:
>> The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (slots).
>> /bin/dmesg | grep -i perf may provide additional information.
>>
>> This is because the stat_handle_error() returns COUNTER_FATAL when the
>> 'slots' is used as leader of events.
>>
>> There are three options to fix the issue.
>>
>> 1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
>> userspace so that pmu_have_event(pmu->name, "slots") returns false.
>>
>> 2. Run cpuid at perf userspace and avoid using 'slots' if it is not
>> supported in cpuid.
>>
>> 3. Do not fatal perf if the leader is failed. Do not create events for an
>> evsel if its leader is already failed.
>>
>> This RFC patch is with the 3rd option. Would you mind suggesting which
>> option is better?
> 
> Sorry for the late reply but I think option 1 is the way to go.
> 
> The option 3 would be a transient workaround and it would affect
> other events too.  If it's really needed, I think option 2 is slightly better
> than option 3.  Or, we can add --force option to skip non-supported
> events explicitly.
> 
> Thanks,
> Namhyung
> 
>>
>> Here is the output of patch.
>>
>> $ perf stat -v
>> Using CPUID GenuineIntel-6-6A-6
>> slots -> cpu/event=0,umask=0x4/
>> topdown-retiring -> cpu/event=0,umask=0x80/
>> topdown-bad-spec -> cpu/event=0,umask=0x81/
>> topdown-fe-bound -> cpu/event=0,umask=0x82/
>> topdown-be-bound -> cpu/event=0,umask=0x83/
>> Control descriptor is not initialized
>> Warning:
>> slots event is not supported by the kernel.
>> ^Ccpu-clock: 62021481051 62021480237 62021480237
>> context-switches: 437 62021478064 62021478064
>> cpu-migrations: 17 62021475294 62021475294
>> page-faults: 12 62021471925 62021471925
>> cycles: 15662273 62020909141 62020909141
>> instructions: 6580385 62008944246 62008944246
>> branches: 1446119 62008855550 62008855550
>> branch-misses: 30970 62008643255 62008643255
>> failed to read counter slots
>> failed to read counter topdown-retiring
>> failed to read counter topdown-bad-spec
>> failed to read counter topdown-fe-bound
>> failed to read counter topdown-be-bound
>>
>>  Performance counter stats for 'system wide':
>>
>>          62,021.48 msec cpu-clock                        #   16.006 CPUs utilized
>>                437      context-switches                 #    7.046 /sec
>>                 17      cpu-migrations                   #    0.274 /sec
>>                 12      page-faults                      #    0.193 /sec
>>         15,662,273      cycles                           #    0.000 GHz
>>          6,580,385      instructions                     #    0.42  insn per cycle
>>          1,446,119      branches                         #   23.316 K/sec
>>             30,970      branch-misses                    #    2.14% of all branches
>>    <not supported>      slots
>>    <not supported>      topdown-retiring
>>    <not supported>      topdown-bad-spec
>>    <not supported>      topdown-fe-bound
>>    <not supported>      topdown-be-bound
>>
>>        3.874991326 seconds time elapsed
>>
>> Thank you very much!
>>
>> Cc: Joe Jin <joe.jin@oracle.com>
>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>> ---
>>  tools/perf/builtin-stat.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 0b4a62e4ff67..1053cf0886c0 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -762,9 +762,7 @@ static enum counter_recovery stat_handle_error(struct evsel *counter)
>>                  */
>>                 counter->errored = true;
>>
>> -               if ((evsel__leader(counter) != counter) ||
>> -                   !(counter->core.leader->nr_members > 1))
>> -                       return COUNTER_SKIP;
>> +               return COUNTER_SKIP;
>>         } else if (evsel__fallback(counter, errno, msg, sizeof(msg))) {
>>                 if (verbose > 0)
>>                         ui__warning("%s\n", msg);
>> @@ -843,6 +841,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>>                 if (target.use_bpf)
>>                         break;
>>
>> +               if (evsel__leader(counter)->errored)
>> +                       continue;
>>                 if (counter->reset_group || counter->errored)
>>                         continue;
>>                 if (evsel__is_bpf(counter))
>> @@ -901,6 +901,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>>                 evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
>>                         counter = evlist_cpu_itr.evsel;
>>
>> +                       if (evsel__leader(counter)->errored)
>> +                               continue;
>>                         if (!counter->reset_group && !counter->errored)
>>                                 continue;
>>                         if (!counter->reset_group)
>> --
>> 2.17.1
>>

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

* Re: [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored
  2022-10-18 21:29   ` Dongli Zhang
@ 2022-10-18 21:31     ` Dongli Zhang
  0 siblings, 0 replies; 10+ messages in thread
From: Dongli Zhang @ 2022-10-18 21:31 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-perf-users, KVM, linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, joe.jin, Like Xu

Please ignore this email. I carelessly clicked unwanted buttons on my
Thunderbird client.

Sorry for spamming.

Dongli Zhang

On 10/18/22 2:29 PM, Dongli Zhang wrote:
> 
> 
> On 10/14/22 3:16 PM, Namhyung Kim wrote:
>> Hello,
>>
>> On Thu, Sep 22, 2022 at 12:10 AM Dongli Zhang <dongli.zhang@oracle.com> wrote:
>>>
>>> Add kvm@vger.kernel.org as this issue is in virtualization env.
>>>
>>> The topdown metrics events became default since
>>> commit 42641d6f4d15 ("perf stat: Add Topdown metrics events as default
>>> events"). The perf will use 'slots' if the
>>> /sys/bus/event_source/devices/cpu/events/slots is available.
>>>
>>> Unfortunately, the 'slots' may not be supported in the virualization
>>> environment. The hypervisor may not expose the 'slots' counter to the VM
>>> in cpuid. As a result, the kernel may disable topdown slots and metrics
>>> events in intel_pmu_init() if slots event is not in CPUID. E.g., both
>>> c->weight and c->idxmsk64 are set to 0.
>>>
>>> There will be below error on Icelake VM since 'slots' is the leader:
>>>
>>> $ perf stat
>>> Error:
>>> The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (slots).
>>> /bin/dmesg | grep -i perf may provide additional information.
>>>
>>> This is because the stat_handle_error() returns COUNTER_FATAL when the
>>> 'slots' is used as leader of events.
>>>
>>> There are three options to fix the issue.
>>>
>>> 1. Do not expose /sys/bus/event_source/devices/cpu/events/slots to
>>> userspace so that pmu_have_event(pmu->name, "slots") returns false.
>>>
>>> 2. Run cpuid at perf userspace and avoid using 'slots' if it is not
>>> supported in cpuid.
>>>
>>> 3. Do not fatal perf if the leader is failed. Do not create events for an
>>> evsel if its leader is already failed.
>>>
>>> This RFC patch is with the 3rd option. Would you mind suggesting which
>>> option is better?
>>
>> Sorry for the late reply but I think option 1 is the way to go.
>>
>> The option 3 would be a transient workaround and it would affect
>> other events too.  If it's really needed, I think option 2 is slightly better
>> than option 3.  Or, we can add --force option to skip non-supported
>> events explicitly.
>>
>> Thanks,
>> Namhyung
>>
>>>
>>> Here is the output of patch.
>>>
>>> $ perf stat -v
>>> Using CPUID GenuineIntel-6-6A-6
>>> slots -> cpu/event=0,umask=0x4/
>>> topdown-retiring -> cpu/event=0,umask=0x80/
>>> topdown-bad-spec -> cpu/event=0,umask=0x81/
>>> topdown-fe-bound -> cpu/event=0,umask=0x82/
>>> topdown-be-bound -> cpu/event=0,umask=0x83/
>>> Control descriptor is not initialized
>>> Warning:
>>> slots event is not supported by the kernel.
>>> ^Ccpu-clock: 62021481051 62021480237 62021480237
>>> context-switches: 437 62021478064 62021478064
>>> cpu-migrations: 17 62021475294 62021475294
>>> page-faults: 12 62021471925 62021471925
>>> cycles: 15662273 62020909141 62020909141
>>> instructions: 6580385 62008944246 62008944246
>>> branches: 1446119 62008855550 62008855550
>>> branch-misses: 30970 62008643255 62008643255
>>> failed to read counter slots
>>> failed to read counter topdown-retiring
>>> failed to read counter topdown-bad-spec
>>> failed to read counter topdown-fe-bound
>>> failed to read counter topdown-be-bound
>>>
>>>  Performance counter stats for 'system wide':
>>>
>>>          62,021.48 msec cpu-clock                        #   16.006 CPUs utilized
>>>                437      context-switches                 #    7.046 /sec
>>>                 17      cpu-migrations                   #    0.274 /sec
>>>                 12      page-faults                      #    0.193 /sec
>>>         15,662,273      cycles                           #    0.000 GHz
>>>          6,580,385      instructions                     #    0.42  insn per cycle
>>>          1,446,119      branches                         #   23.316 K/sec
>>>             30,970      branch-misses                    #    2.14% of all branches
>>>    <not supported>      slots
>>>    <not supported>      topdown-retiring
>>>    <not supported>      topdown-bad-spec
>>>    <not supported>      topdown-fe-bound
>>>    <not supported>      topdown-be-bound
>>>
>>>        3.874991326 seconds time elapsed
>>>
>>> Thank you very much!
>>>
>>> Cc: Joe Jin <joe.jin@oracle.com>
>>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>>> ---
>>>  tools/perf/builtin-stat.c | 8 +++++---
>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>>> index 0b4a62e4ff67..1053cf0886c0 100644
>>> --- a/tools/perf/builtin-stat.c
>>> +++ b/tools/perf/builtin-stat.c
>>> @@ -762,9 +762,7 @@ static enum counter_recovery stat_handle_error(struct evsel *counter)
>>>                  */
>>>                 counter->errored = true;
>>>
>>> -               if ((evsel__leader(counter) != counter) ||
>>> -                   !(counter->core.leader->nr_members > 1))
>>> -                       return COUNTER_SKIP;
>>> +               return COUNTER_SKIP;
>>>         } else if (evsel__fallback(counter, errno, msg, sizeof(msg))) {
>>>                 if (verbose > 0)
>>>                         ui__warning("%s\n", msg);
>>> @@ -843,6 +841,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>>>                 if (target.use_bpf)
>>>                         break;
>>>
>>> +               if (evsel__leader(counter)->errored)
>>> +                       continue;
>>>                 if (counter->reset_group || counter->errored)
>>>                         continue;
>>>                 if (evsel__is_bpf(counter))
>>> @@ -901,6 +901,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
>>>                 evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
>>>                         counter = evlist_cpu_itr.evsel;
>>>
>>> +                       if (evsel__leader(counter)->errored)
>>> +                               continue;
>>>                         if (!counter->reset_group && !counter->errored)
>>>                                 continue;
>>>                         if (!counter->reset_group)
>>> --
>>> 2.17.1
>>>

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

end of thread, other threads:[~2022-10-18 21:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22  7:10 [PATCH RFC 1/1] perf stat: do not fatal if the leader is errored Dongli Zhang
2022-09-22  8:07 ` Like Xu
2022-09-22 13:34   ` Liang, Kan
2022-09-22 18:00     ` Dongli Zhang
2022-09-22 18:42       ` Liang, Kan
2022-09-22 18:10   ` Dongli Zhang
2022-10-14 22:16 ` Namhyung Kim
2022-10-14 23:47   ` Dongli Zhang
2022-10-18 21:29   ` Dongli Zhang
2022-10-18 21:31     ` Dongli Zhang

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