perf/x86/intel: Correct wrong 'or' operation for pmu capabilities

Message ID 20231121014628.729989-1-dapeng1.mi@linux.intel.com
State New
Headers
Series perf/x86/intel: Correct wrong 'or' operation for pmu capabilities |

Commit Message

Mi, Dapeng Nov. 21, 2023, 1:46 a.m. UTC
  When running perf-stat command on Intel hybrid platform, perf-stat
reports the following errors.

sudo taskset -c 7 ./perf stat -vvvv -e cpu_atom/instructions/ sleep 1

Opening: cpu/cycles/:HG
------------------------------------------------------------
perf_event_attr:
  type                             0 (PERF_TYPE_HARDWARE)
  config                           0xa00000000
  disabled                         1
------------------------------------------------------------
sys_perf_event_open: pid 0  cpu -1  group_fd -1  flags 0x8
sys_perf_event_open failed, error -16

 Performance counter stats for 'sleep 1':

     <not counted>      cpu_atom/instructions/

It looks the cpu_atom/instructions/ event can't be enabled on atom pmu
even the process is pinned on atom core. The investigation shows that
exclusive_event_init() helper always returns -EBUSY error in the perf
event creation. That's strange since the atom pmu should not be a
exclusive PMU.

Further investigation shows the issue is introduced by commit 97588df87b56
("perf/x86/intel: Add common intel_pmu_init_hybrid()"). The commit
originally intents to clear the bit PERF_PMU_CAP_AUX_OUTPUT from pmu
capabilities if intel_cap.pebs_output_pt_available is not set, but it
incorrectly uses 'or' operation and leads to all pmu capabilities bits
are set to 1 except bit PERF_PMU_CAP_AUX_OUTPUT.

Testing this fix on Intel hybrid platforms, the observed issues
disappear.

Fixes: 97588df87b56 ("perf/x86/intel: Add common intel_pmu_init_hybrid()")
Cc: stable@vger.kernel.org
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
 arch/x86/events/intel/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
  

Comments

Liang, Kan Nov. 21, 2023, 1:52 p.m. UTC | #1
On 2023-11-20 8:46 p.m., Dapeng Mi wrote:
> When running perf-stat command on Intel hybrid platform, perf-stat
> reports the following errors.
> 
> sudo taskset -c 7 ./perf stat -vvvv -e cpu_atom/instructions/ sleep 1
> 
> Opening: cpu/cycles/:HG
> ------------------------------------------------------------
> perf_event_attr:
>   type                             0 (PERF_TYPE_HARDWARE)
>   config                           0xa00000000
>   disabled                         1
> ------------------------------------------------------------
> sys_perf_event_open: pid 0  cpu -1  group_fd -1  flags 0x8
> sys_perf_event_open failed, error -16
> 
>  Performance counter stats for 'sleep 1':
> 
>      <not counted>      cpu_atom/instructions/
> 
> It looks the cpu_atom/instructions/ event can't be enabled on atom pmu
> even the process is pinned on atom core. The investigation shows that
> exclusive_event_init() helper always returns -EBUSY error in the perf
> event creation. That's strange since the atom pmu should not be a
> exclusive PMU.
> 
> Further investigation shows the issue is introduced by commit 97588df87b56
> ("perf/x86/intel: Add common intel_pmu_init_hybrid()"). The commit
> originally intents to clear the bit PERF_PMU_CAP_AUX_OUTPUT from pmu
> capabilities if intel_cap.pebs_output_pt_available is not set, but it
> incorrectly uses 'or' operation and leads to all pmu capabilities bits
> are set to 1 except bit PERF_PMU_CAP_AUX_OUTPUT.
> 
> Testing this fix on Intel hybrid platforms, the observed issues
> disappear.
> 
> Fixes: 97588df87b56 ("perf/x86/intel: Add common intel_pmu_init_hybrid()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

Reviewed-by: Kan Liang <kan.liang@linux.intel.com>

Thanks,
Kan

> ---
>  arch/x86/events/intel/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index a08f794a0e79..ce1c777227b4 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -4660,7 +4660,7 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
>  	if (pmu->intel_cap.pebs_output_pt_available)
>  		pmu->pmu.capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
>  	else
> -		pmu->pmu.capabilities |= ~PERF_PMU_CAP_AUX_OUTPUT;
> +		pmu->pmu.capabilities &= ~PERF_PMU_CAP_AUX_OUTPUT;
>  
>  	intel_pmu_check_event_constraints(pmu->event_constraints,
>  					  pmu->num_counters,
> 
> base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
  

Patch

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index a08f794a0e79..ce1c777227b4 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4660,7 +4660,7 @@  static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
 	if (pmu->intel_cap.pebs_output_pt_available)
 		pmu->pmu.capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
 	else
-		pmu->pmu.capabilities |= ~PERF_PMU_CAP_AUX_OUTPUT;
+		pmu->pmu.capabilities &= ~PERF_PMU_CAP_AUX_OUTPUT;
 
 	intel_pmu_check_event_constraints(pmu->event_constraints,
 					  pmu->num_counters,