Skip to contentSkip to conversation

dmaengine:idxd: Use local64_try_cmpxchg in perfmon_pmu_event_update

PATCH Applied Uros Bizjak 4 participants 4 messages Jul 3, 2023 – Aug 1, 2023

Subsystems (2)
dmaengine Intel IDXD driver
lore
Share & tools
lore

Uros Bizjak

author
#0 lore raw

PATCH 1/1 dmaengine:idxd: Use local64_try_cmpxchg in perfmon_pmu_event_update

Use local64_try_cmpxchg instead of local64_cmpxchg (*ptr, old, new) == old in perfmon_pmu_event_update. x86 CMPXCHG instruction returns success in ZF flag, so this change saves a compare after cmpxchg (and related move instruction in front of cmpxchg).

Also, try_cmpxchg implicitly assigns old *ptr value to "old" when cmpxchg fails. There is no need to re-read the value in the loop.

No functional change intended.

Cc Fenghua Yu <[email protected]>
Cc Dave Jiang <[email protected]>
Cc Vinod Koul <[email protected]>
Signed-off-by Uros Bizjak <[email protected]>
---
 drivers/dma/idxd/perfmon.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/dma/idxd/perfmon.c +3 -4
index d73004f47cf4..fdda6d604262 100644
@@ -245,12 +245,11 @@ static void perfmon_pmu_event_update(struct perf_event *event)
 	int shift = 64 - idxd->idxd_pmu->counter_width;
 	struct hw_perf_event *hwc = &event->hw;
 
+	prev_raw_count = local64_read(&hwc->prev_count);
 	do {
-		prev_raw_count = local64_read(&hwc->prev_count);
 		new_raw_count = perfmon_pmu_read_counter(event);
-	} while (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
-			new_raw_count) != prev_raw_count);
-
+	} while (!local64_try_cmpxchg(&hwc->prev_count,
+				      &prev_raw_count, new_raw_count));
 	n = (new_raw_count << shift);
 	p = (prev_raw_count << shift);
Signature · 1 line
2.41.0
reviewer
#1 lore raw
Quoted 8 lines · Uros Bizjak

Use local64_try_cmpxchg instead of local64_cmpxchg (*ptr, old, new) == old in perfmon_pmu_event_update. x86 CMPXCHG instruction returns success in ZF flag, so this change saves a compare after cmpxchg (and related move instruction in front of cmpxchg).

Also, try_cmpxchg implicitly assigns old *ptr value to "old" when cmpxchg fails. There is no need to re-read the value in the loop.

Quoted 6 lines

No functional change intended.

Cc Fenghua Yu <[email protected]>
Cc Dave Jiang <[email protected]>
Cc Vinod Koul <[email protected]>
Signed-off-by Uros Bizjak <[email protected]>
Cc Tom Zanussi

Tom do you mind review this patch? Thanks!

Quoted 24 lines
---
  drivers/dma/idxd/perfmon.c | 7 +++----
  1 file changed, 3 insertions(+), 4 deletions(-)
drivers/dma/idxd/perfmon.c +3 -4
index d73004f47cf4..fdda6d604262 100644
@@ -245,12 +245,11 @@ static void perfmon_pmu_event_update(struct perf_event *event)
  	int shift = 64 - idxd->idxd_pmu->counter_width;
  	struct hw_perf_event *hwc = &event->hw;
 
+	prev_raw_count = local64_read(&hwc->prev_count);
  	do {
-		prev_raw_count = local64_read(&hwc->prev_count);
  		new_raw_count = perfmon_pmu_read_counter(event);
-	} while (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
-			new_raw_count) != prev_raw_count);
-
+	} while (!local64_try_cmpxchg(&hwc->prev_count,
+				      &prev_raw_count, new_raw_count));
  	n = (new_raw_count << shift);
  	p = (prev_raw_count << shift);
Reviewed-by
#2 lore raw
Quoted 19 lines · Dave Jiang
Quoted 8 lines · Uros Bizjak

Use local64_try_cmpxchg instead of local64_cmpxchg (*ptr, old, new) == old in perfmon_pmu_event_update.  x86 CMPXCHG instruction returns success in ZF flag, so this change saves a compare after cmpxchg (and related move instruction in front of cmpxchg).

Also, try_cmpxchg implicitly assigns old *ptr value to "old" when cmpxchg fails. There is no need to re-read the value in the loop.

Quoted 6 lines

No functional change intended.

Cc Fenghua Yu <[email protected]>
Cc Dave Jiang <[email protected]>
Cc Vinod Koul <[email protected]>
Signed-off-by Uros Bizjak <[email protected]>
Quoted 3 lines
Cc Tom Zanussi

Tom do you mind review this patch? Thanks!

Looks fine to me.

Thanks,

Reviewed-by Tom Zanussi <[email protected]>
Quoted 25 lines
Quoted 25 lines

---
  drivers/dma/idxd/perfmon.c | 7 +++----

  1 file changed, 3 insertions(+), 4 deletions(-)
drivers/dma/idxd/perfmon.c +0 -0
index d73004f47cf4..fdda6d604262 100644
@@ -245,12 +245,11 @@ static void perfmon_pmu_event_update(struct perf_event *event)
         int shift = 64 - idxd->idxd_pmu->counter_width;
         struct hw_perf_event *hwc = &event->hw;


+       prev_raw_count = local64_read(&hwc->prev_count);
        do {
-               prev_raw_count = local64_read(&hwc->prev_count); new_raw_count = perfmon_pmu_read_counter(event); -       } while (local64_cmpxchg(&hwc->prev_count, prev_raw_count, -                       new_raw_count) != prev_raw_count);
-
+       } while (!local64_try_cmpxchg(&hwc->prev_count,
+                                     &prev_raw_count, new_raw_count)); n = (new_raw_count << shift);
        p = (prev_raw_count << shift);

maintainer Applied
#3 lore raw
In reply to Uros Bizjak
Quoted 9 lines · Uros Bizjak

Use local64_try_cmpxchg instead of local64_cmpxchg (*ptr, old, new) == old in perfmon_pmu_event_update. x86 CMPXCHG instruction returns success in ZF flag, so this change saves a compare after cmpxchg (and related move instruction in front of cmpxchg).

Also, try_cmpxchg implicitly assigns old *ptr value to "old" when cmpxchg fails. There is no need to re-read the value in the loop.

[...]

Applied, thanks!

[1/1] dmaengine:idxd: Use local64_try_cmpxchg in perfmon_pmu_event_update

      commit: cae701b9ccf128edea26982f73178087fc3ad180

Best regards,

Signature · 1 line
~Vinod