perf/x86/core: Remove repeating test expression

Message ID Y/yYTY7c7L0+d+Gb@ubun2204.myguest.virtualbox.org
State New
Headers
Series perf/x86/core: Remove repeating test expression |

Commit Message

Deepak R Varma Feb. 27, 2023, 11:47 a.m. UTC
  Current implementation already checks validity of the cpu_type for the
hybrid pmu two lines above. Hence there is no need to again include
it in the immediate if test evaluation.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 arch/x86/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Deepak R Varma Feb. 27, 2023, 12:27 p.m. UTC | #1
On Mon, Feb 27, 2023 at 05:17:25PM +0530, Deepak R Varma wrote:
> Current implementation already checks validity of the cpu_type for the
> hybrid pmu two lines above. Hence there is no need to again include
> it in the immediate if test evaluation.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---

Please disregard. I misread the bitwise operation as logical evaluation. Sorry
for any inconvenience.

Deepak.

>  arch/x86/events/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 87a7f0cd77fd..89db2352deb9 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -1893,7 +1893,7 @@ ssize_t events_hybrid_sysfs_show(struct device *dev,
>  	for (i = 0; i < x86_pmu.num_hybrid_pmus; i++) {
>  		if (!(x86_pmu.hybrid_pmu[i].cpu_type & pmu_attr->pmu_type))
>  			continue;
> -		if (x86_pmu.hybrid_pmu[i].cpu_type & pmu->cpu_type) {
> +		if (pmu->cpu_type) {
>  			next_str = strchr(str, ';');
>  			if (next_str)
>  				return snprintf(page, next_str - str + 1, "%s", str);
> -- 
> 2.34.1
>
  
Liang, Kan Feb. 27, 2023, 4:20 p.m. UTC | #2
On 2023-02-27 6:47 a.m., Deepak R Varma wrote:
> Current implementation already checks validity of the cpu_type for the
> hybrid pmu two lines above. Hence there is no need to again include
> it in the immediate if test evaluation.

The pmu_attr is different from the pmu. The pmu_attr is the
EVENT_ATTR_STR_HYBRID() which is defined in the kernel. The pmu is
calculated from the sysfs PMU entry. The first two line is to check
whether the attr is defined in the kernel. The second check is to find
the correct attr for the sysfs PMU entry. I don't think we should remove
the second check.

Thanks,
Kan
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
>  arch/x86/events/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 87a7f0cd77fd..89db2352deb9 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -1893,7 +1893,7 @@ ssize_t events_hybrid_sysfs_show(struct device *dev,
>  	for (i = 0; i < x86_pmu.num_hybrid_pmus; i++) {
>  		if (!(x86_pmu.hybrid_pmu[i].cpu_type & pmu_attr->pmu_type))
>  			continue;
> -		if (x86_pmu.hybrid_pmu[i].cpu_type & pmu->cpu_type) {
> +		if (pmu->cpu_type) {
>  			next_str = strchr(str, ';');
>  			if (next_str)
>  				return snprintf(page, next_str - str + 1, "%s", str);
  

Patch

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 87a7f0cd77fd..89db2352deb9 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -1893,7 +1893,7 @@  ssize_t events_hybrid_sysfs_show(struct device *dev,
 	for (i = 0; i < x86_pmu.num_hybrid_pmus; i++) {
 		if (!(x86_pmu.hybrid_pmu[i].cpu_type & pmu_attr->pmu_type))
 			continue;
-		if (x86_pmu.hybrid_pmu[i].cpu_type & pmu->cpu_type) {
+		if (pmu->cpu_type) {
 			next_str = strchr(str, ';');
 			if (next_str)
 				return snprintf(page, next_str - str + 1, "%s", str);