Skip to contentSkip to conversation

ARM: perf: move active_events into struct arm_pmu

RFC PATCH New Ashwin Chaugule 2 participants 2 messages Aug 22, 2011 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 4B9A4BAF850C914D8DED94776A2C477E073D90C2@nasanexd01f.na.qualcomm.com

Ashwin Chaugule

author
#1 lore raw

RFC PATCH 5/15 ARM: perf: move active_events into struct arm_pmu

Hi Mark,

Quoted 5 lines
Quoted 5 lines

From: Mark Rutland
case PERF_TYPE_RAW:

(quoted diff) +0 -0
@@ -556,15 +559,14 @@ static int armpmu_event_init(struct perf_event
 *event)
Quoted 8 lines

Review static int armpmu_event_init(struct perf_event · line 565

Quoted 8 lines
(quoted diff) +3 -3
 	event->destroy = hw_perf_event_destroy;
 
-	if (!atomic_inc_not_zero(&active_events)) {
-		mutex_lock(&pmu_reserve_mutex);
-		if (atomic_read(&active_events) == 0) {
+	if (!atomic_inc_not_zero(active_events)) {
+		mutex_lock(&armpmu->reserve_mutex);
+		if (atomic_read(active_events) == 0)

Isn't this use of atomic_* still racy ?

http://www.spinics.net/lists/arm-kernel/msg123297.html

Cheers,
Ashwin

Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
#2 lore raw

RFC PATCH 5/15 ARM: perf: move active_events into struct arm_pmu

Ashwin,

Quoted 9 lines · Ashwin Chaugule

Hi Mark,

Quoted 7 lines
Quoted 7 lines

From: Mark Rutland
case PERF_TYPE_RAW:

(quoted diff) +0 -0
@@ -556,15 +559,14 @@ static int armpmu_event_init(struct perf_event
 *event)
 
 	event->destroy = hw_perf_event_destroy;
Quoted 9 lines
Quoted 6 lines

Review static int armpmu_event_init(struct perf_event · line 565

Quoted 6 lines
(quoted diff) +3 -3
-	if (!atomic_inc_not_zero(&active_events)) {
-		mutex_lock(&pmu_reserve_mutex);
-		if (atomic_read(&active_events) == 0) {
+	if (!atomic_inc_not_zero(active_events)) {
+		mutex_lock(&armpmu->reserve_mutex);
+		if (atomic_read(active_events) == 0)

Isn't this use of atomic_* still racy ?

I don't believe so.

Quoted 1 line

http://www.spinics.net/lists/arm-kernel/msg123297.html

That thread is referring to previous behaviour where it was assumed that a group of atomic operations would execute as an atomic block. The code in question was fixed in 57ce9bb3 ("ARM: 6902/1: perf: Remove erroneous check on active_events").

Now that we only modify active_events with the reserve_mutex held, we're safe as houses.

Will