[PATCH] perf/x86/amd: Use try_cmpxchg() in events/amd/{un,}core.c

Uros Bizjak posted 1 patch 2 years, 2 months ago
arch/x86/events/amd/core.c   | 3 ++-
arch/x86/events/amd/uncore.c | 6 ++++--
2 files changed, 6 insertions(+), 3 deletions(-)
[PATCH] perf/x86/amd: Use try_cmpxchg() in events/amd/{un,}core.c
Posted by Uros Bizjak 2 years, 2 months ago
Replace this pattern in events/amd/{un,}core.c:

    cmpxchg(*ptr, old, new) == old

... with the simpler and faster:

    try_cmpxchg(*ptr, &old, new)

The x86 CMPXCHG instruction returns success in the ZF flag, so this change
saves a compare after the CMPXCHG.

No functional change intended.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/events/amd/core.c   | 3 ++-
 arch/x86/events/amd/uncore.c | 6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 1fc4ce44e743..c1b8aeff7bc0 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -433,7 +433,8 @@ static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
 	 * when we come here
 	 */
 	for (i = 0; i < x86_pmu.num_counters; i++) {
-		if (cmpxchg(nb->owners + i, event, NULL) == event)
+		struct perf_event *tmp = event;
+		if (try_cmpxchg(nb->owners + i, &tmp, NULL))
 			break;
 	}
 }
diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index 4ccb8fa483e6..cac7c20d1aee 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -162,7 +162,8 @@ static int amd_uncore_add(struct perf_event *event, int flags)
 	/* if not, take the first available counter */
 	hwc->idx = -1;
 	for (i = 0; i < pmu->num_counters; i++) {
-		if (cmpxchg(&ctx->events[i], NULL, event) == NULL) {
+		struct perf_event *tmp = NULL;
+		if (try_cmpxchg(&ctx->events[i], &tmp, event)) {
 			hwc->idx = i;
 			break;
 		}
@@ -196,7 +197,8 @@ static void amd_uncore_del(struct perf_event *event, int flags)
 	event->pmu->stop(event, PERF_EF_UPDATE);
 
 	for (i = 0; i < pmu->num_counters; i++) {
-		if (cmpxchg(&ctx->events[i], event, NULL) == event)
+		struct perf_event *tmp = event;
+		if (try_cmpxchg(&ctx->events[i], &tmp, NULL))
 			break;
 	}
 
-- 
2.44.0
Re: [PATCH] perf/x86/amd: Use try_cmpxchg() in events/amd/{un,}core.c
Posted by Ingo Molnar 2 years, 1 month ago
* Uros Bizjak <ubizjak@gmail.com> wrote:

> Replace this pattern in events/amd/{un,}core.c:
> 
>     cmpxchg(*ptr, old, new) == old
> 
> ... with the simpler and faster:
> 
>     try_cmpxchg(*ptr, &old, new)
> 
> The x86 CMPXCHG instruction returns success in the ZF flag, so this change
> saves a compare after the CMPXCHG.
> 
> No functional change intended.
> 
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> ---
>  arch/x86/events/amd/core.c   | 3 ++-
>  arch/x86/events/amd/uncore.c | 6 ++++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index 1fc4ce44e743..c1b8aeff7bc0 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -433,7 +433,8 @@ static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
>  	 * when we come here
>  	 */
>  	for (i = 0; i < x86_pmu.num_counters; i++) {
> -		if (cmpxchg(nb->owners + i, event, NULL) == event)
> +		struct perf_event *tmp = event;
> +		if (try_cmpxchg(nb->owners + i, &tmp, NULL))
>  			break;
>  	}
>  }
> diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
> index 4ccb8fa483e6..cac7c20d1aee 100644
> --- a/arch/x86/events/amd/uncore.c
> +++ b/arch/x86/events/amd/uncore.c
> @@ -162,7 +162,8 @@ static int amd_uncore_add(struct perf_event *event, int flags)
>  	/* if not, take the first available counter */
>  	hwc->idx = -1;
>  	for (i = 0; i < pmu->num_counters; i++) {
> -		if (cmpxchg(&ctx->events[i], NULL, event) == NULL) {
> +		struct perf_event *tmp = NULL;
> +		if (try_cmpxchg(&ctx->events[i], &tmp, event)) {
>  			hwc->idx = i;
>  			break;
>  		}
> @@ -196,7 +197,8 @@ static void amd_uncore_del(struct perf_event *event, int flags)
>  	event->pmu->stop(event, PERF_EF_UPDATE);
>  
>  	for (i = 0; i < pmu->num_counters; i++) {
> -		if (cmpxchg(&ctx->events[i], event, NULL) == event)
> +		struct perf_event *tmp = event;
> +		if (try_cmpxchg(&ctx->events[i], &tmp, NULL))
>  			break;
>  	}

Note that's a newline missing between local variable declaration block and 
the first statement after that. I've added all 3 to this patch (see below), 
but please keep an eye on this in future patches.

Thanks,

	Ingo

================>
 arch/x86/events/amd/core.c   | 1 +
 arch/x86/events/amd/uncore.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index c1b8aeff7bc0..18bfe3451f3a 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -434,6 +434,7 @@ static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
 	 */
 	for (i = 0; i < x86_pmu.num_counters; i++) {
 		struct perf_event *tmp = event;
+
 		if (try_cmpxchg(nb->owners + i, &tmp, NULL))
 			break;
 	}
diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index cac7c20d1aee..0fafe233bba4 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -163,6 +163,7 @@ static int amd_uncore_add(struct perf_event *event, int flags)
 	hwc->idx = -1;
 	for (i = 0; i < pmu->num_counters; i++) {
 		struct perf_event *tmp = NULL;
+
 		if (try_cmpxchg(&ctx->events[i], &tmp, event)) {
 			hwc->idx = i;
 			break;
@@ -198,6 +199,7 @@ static void amd_uncore_del(struct perf_event *event, int flags)
 
 	for (i = 0; i < pmu->num_counters; i++) {
 		struct perf_event *tmp = event;
+
 		if (try_cmpxchg(&ctx->events[i], &tmp, NULL))
 			break;
 	}
[tip: perf/urgent] perf/x86/amd: Use try_cmpxchg() in events/amd/{un,}core.c
Posted by tip-bot2 for Uros Bizjak 2 years, 1 month ago
The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     cd84351c8c1baec86342d784feb884ace007d51c
Gitweb:        https://git.kernel.org/tip/cd84351c8c1baec86342d784feb884ace007d51c
Author:        Uros Bizjak <ubizjak@gmail.com>
AuthorDate:    Thu, 25 Apr 2024 12:16:14 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Sat, 18 May 2024 11:15:13 +02:00

perf/x86/amd: Use try_cmpxchg() in events/amd/{un,}core.c

Replace this pattern in events/amd/{un,}core.c:

    cmpxchg(*ptr, old, new) == old

... with the simpler and faster:

    try_cmpxchg(*ptr, &old, new)

The x86 CMPXCHG instruction returns success in the ZF flag, so this change
saves a compare after the CMPXCHG.

No functional change intended.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20240425101708.5025-1-ubizjak@gmail.com
---
 arch/x86/events/amd/core.c   | 4 +++-
 arch/x86/events/amd/uncore.c | 8 ++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 1fc4ce4..18bfe34 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -433,7 +433,9 @@ static void __amd_put_nb_event_constraints(struct cpu_hw_events *cpuc,
 	 * when we come here
 	 */
 	for (i = 0; i < x86_pmu.num_counters; i++) {
-		if (cmpxchg(nb->owners + i, event, NULL) == event)
+		struct perf_event *tmp = event;
+
+		if (try_cmpxchg(nb->owners + i, &tmp, NULL))
 			break;
 	}
 }
diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index 4ccb8fa..0fafe23 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -162,7 +162,9 @@ static int amd_uncore_add(struct perf_event *event, int flags)
 	/* if not, take the first available counter */
 	hwc->idx = -1;
 	for (i = 0; i < pmu->num_counters; i++) {
-		if (cmpxchg(&ctx->events[i], NULL, event) == NULL) {
+		struct perf_event *tmp = NULL;
+
+		if (try_cmpxchg(&ctx->events[i], &tmp, event)) {
 			hwc->idx = i;
 			break;
 		}
@@ -196,7 +198,9 @@ static void amd_uncore_del(struct perf_event *event, int flags)
 	event->pmu->stop(event, PERF_EF_UPDATE);
 
 	for (i = 0; i < pmu->num_counters; i++) {
-		if (cmpxchg(&ctx->events[i], event, NULL) == event)
+		struct perf_event *tmp = event;
+
+		if (try_cmpxchg(&ctx->events[i], &tmp, NULL))
 			break;
 	}