oprofile: perf: use NR_CPUS instead or nr_cpumask_bits for static array

The OProfile perf backend uses a static array to keep track of the perf events on the system. When compiling with CONFIG_CPUMASK_OFFSTACK=y && SMP, nr_cpumask_bits is not a compile-time constant and the build will fail with:

oprofile_perf.c:28: error: variably modified 'perf_events' at file scope

This patch uses NR_CPUs instead of nr_cpumask_bits for the array initialisation. If this causes space problems in the future, we can always move to dynamic allocation for the events array.

Cc Matt Fleming <[email protected]>

Cc Robert Richter <[email protected]>

Reported-by Russell King - ARM Linux <[email protected]>

Signed-off-by Will Deacon <[email protected]>

---
 drivers/oprofile/oprofile_perf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/oprofile/oprofile_perf.c +1 -1

index da14432..efc4b7f 100644
@@ -25,7 +25,7 @@ static int oprofile_perf_enabled;
 static DEFINE_MUTEX(oprofile_perf_mutex);
 
 static struct op_counter_config *counter_config;
-static struct perf_event **perf_events[nr_cpumask_bits];
+static struct perf_event **perf_events[NR_CPUS];
 static int num_counters;
 
 /*

Signature · 1 line

1.7.4.1

Given both the lack of feedback and the trivial nature of this patch would you be able to pick this up please? I'd rather not push it via the ARM tree as the change is under drivers/.

Quoted 3 lines · Will Deacon

Given both the lack of feedback and the trivial nature of this patch would you be able to pick this up please? I'd rather not push it via the ARM tree as the change is under drivers/.

Will, I will apply it. Sorry for not responding earlier.

Quoted 6 lines · Robert Richter

Quoted 3 lines · Will Deacon

Given both the lack of feedback and the trivial nature of this patch would you be able to pick this up please? I'd rather not push it via the ARM tree as the change is under drivers/.

Will, I will apply it. Sorry for not responding earlier.

From f8bbfd7d28303967ca4e8597de9bdc9bf8b197e7 Mon Sep 17 00:00:00 2001

Will,

I just realized I wrote a patch some time ago that introduces per_cpu variables, see below. Please give it a try to, I will then queue it for v3.6.

Will,

Quoted 7 lines · Will Deacon

Quoted 6 lines · Robert Richter

Quoted 3 lines · Will Deacon

Given both the lack of feedback and the trivial nature of this patch would you be able to pick this up please? I'd rather not push it via the ARM tree as the change is under drivers/.

Will, I will apply it. Sorry for not responding earlier.

I just realized I wrote a patch some time ago that introduces per_cpu variables, see below. Please give it a try to, I will then queue it for v3.6.

Your patch should go upstream anyway to make it into stable trees.

Thanks,

-Robert

From f8bbfd7d28303967ca4e8597de9bdc9bf8b197e7 Mon Sep 17 00:00:00 2001
From: Robert Richter <[email protected]>
Date: Thu, 23 Feb 2012 17:07:06 +0100
Subject: [PATCH] oprofile, perf: Use per-cpu framework

This changes oprofile_perf.c to use the per-cpu framework.

Using the per-cpu framework should avoid error like the following:

arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:28:28: error: variably modified 'perf_events' at file scope

Reported-by William Cohen <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Robert Richter <[email protected]>

---
 drivers/oprofile/oprofile_perf.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

drivers/oprofile/oprofile_perf.c +11 -12

index efc4b7f..f3cfa0b 100644
@@ -1,5 +1,6 @@
 /*
  * Copyright 2010 ARM Ltd.
+ * Copyright 2012 Advanced Micro Devices, Inc., Robert Richter
  *
  * Perf-events backend for OProfile.
  */

@@ -25,7 +26,7 @@ static int oprofile_perf_enabled;
 static DEFINE_MUTEX(oprofile_perf_mutex);
 
 static struct op_counter_config *counter_config;
-static struct perf_event **perf_events[NR_CPUS];
+static DEFINE_PER_CPU(struct perf_event **, perf_events);
 static int num_counters;
 
 /*

@@ -38,7 +39,7 @@ static void op_overflow_handler(struct perf_event *event,
 	u32 cpu = smp_processor_id();
 
 	for (id = 0; id < num_counters; ++id)
-		if (perf_events[cpu][id] == event)
+		if (per_cpu(perf_events, cpu)[id] == event)
 			break;
 
 	if (id != num_counters)

@@ -74,7 +75,7 @@ static int op_create_counter(int cpu, int event)
 {
 	struct perf_event *pevent;
 
-	if (!counter_config[event].enabled || perf_events[cpu][event])
+	if (!counter_config[event].enabled || per_cpu(perf_events, cpu)[event])
 		return 0;
 
 	pevent = perf_event_create_kernel_counter(&counter_config[event].attr,

@@ -91,18 +92,18 @@ static int op_create_counter(int cpu, int event)
 		return -EBUSY;
 	}
 
-	perf_events[cpu][event] = pevent;
+	per_cpu(perf_events, cpu)[event] = pevent;
 
 	return 0;
 }
 
 static void op_destroy_counter(int cpu, int event)
 {
-	struct perf_event *pevent = perf_events[cpu][event];
+	struct perf_event *pevent = per_cpu(perf_events, cpu)[event];
 
 	if (pevent) {
 		perf_event_release_kernel(pevent);
-		perf_events[cpu][event] = NULL;
+		per_cpu(perf_events, cpu)[event] = NULL;
 	}
 }
 

@@ -257,12 +258,12 @@ void oprofile_perf_exit(void)
 
 	for_each_possible_cpu(cpu) {
 		for (id = 0; id < num_counters; ++id) {
-			event = perf_events[cpu][id];
+			event = per_cpu(perf_events, cpu)[id];
 			if (event)
 				perf_event_release_kernel(event);
 		}
 
-		kfree(perf_events[cpu]);
+		kfree(per_cpu(perf_events, cpu));
 	}
 
 	kfree(counter_config);

@@ -277,8 +278,6 @@ int __init oprofile_perf_init(struct oprofile_operations *ops)
 	if (ret)
 		return ret;
 
-	memset(&perf_events, 0, sizeof(perf_events));
-
 	num_counters = perf_num_counters();
 	if (num_counters <= 0) {
 		pr_info("oprofile: no performance counters\n");

@@ -298,9 +297,9 @@ int __init oprofile_perf_init(struct oprofile_operations *ops)
 	}
 
 	for_each_possible_cpu(cpu) {
-		perf_events[cpu] = kcalloc(num_counters,
+		per_cpu(perf_events, cpu) = kcalloc(num_counters,
 				sizeof(struct perf_event *), GFP_KERNEL);
-		if (!perf_events[cpu]) {
+		if (!per_cpu(perf_events, cpu)) {
 			pr_info("oprofile: failed to allocate %d perf events "
 					"for cpu %d\n", num_counters, cpu);
 			ret = -ENOMEM;

Will,

Quoted 7 lines · Will Deacon

Quoted 6 lines · Robert Richter

Quoted 3 lines · Will Deacon

Given both the lack of feedback and the trivial nature of this patch would you be able to pick this up please? I'd rather not push it via the ARM tree as the change is under drivers/.

Will, I will apply it. Sorry for not responding earlier.

I just realized I wrote a patch some time ago that introduces per_cpu variables, see below. Please give it a try to, I will then queue it for v3.6.

Your patch should go upstream anyway to make it into stable trees.

Thanks,

-Robert

From f8bbfd7d28303967ca4e8597de9bdc9bf8b197e7 Mon Sep 17 00:00:00 2001
From: Robert Richter <[email protected]>
Date: Thu, 23 Feb 2012 17:07:06 +0100
Subject: [PATCH] oprofile, perf: Use per-cpu framework

This changes oprofile_perf.c to use the per-cpu framework.

Using the per-cpu framework should avoid error like the following:

arch/arm/oprofile/../../../drivers/oprofile/oprofile_perf.c:28:28: error: variably modified 'perf_events' at file scope

Reported-by William Cohen <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Robert Richter <[email protected]>

---
 drivers/oprofile/oprofile_perf.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

drivers/oprofile/oprofile_perf.c +11 -12

index efc4b7f..f3cfa0b 100644
@@ -1,5 +1,6 @@
 /*
  * Copyright 2010 ARM Ltd.
+ * Copyright 2012 Advanced Micro Devices, Inc., Robert Richter
  *
  * Perf-events backend for OProfile.
  */

@@ -25,7 +26,7 @@ static int oprofile_perf_enabled;
 static DEFINE_MUTEX(oprofile_perf_mutex);
 
 static struct op_counter_config *counter_config;
-static struct perf_event **perf_events[NR_CPUS];
+static DEFINE_PER_CPU(struct perf_event **, perf_events);
 static int num_counters;
 
 /*

@@ -38,7 +39,7 @@ static void op_overflow_handler(struct perf_event *event,
 	u32 cpu = smp_processor_id();
 
 	for (id = 0; id < num_counters; ++id)
-		if (perf_events[cpu][id] == event)
+		if (per_cpu(perf_events, cpu)[id] == event)
 			break;
 
 	if (id != num_counters)

@@ -74,7 +75,7 @@ static int op_create_counter(int cpu, int event)
 {
 	struct perf_event *pevent;
 
-	if (!counter_config[event].enabled || perf_events[cpu][event])
+	if (!counter_config[event].enabled || per_cpu(perf_events, cpu)[event])
 		return 0;
 
 	pevent = perf_event_create_kernel_counter(&counter_config[event].attr,

@@ -91,18 +92,18 @@ static int op_create_counter(int cpu, int event)
 		return -EBUSY;
 	}
 
-	perf_events[cpu][event] = pevent;
+	per_cpu(perf_events, cpu)[event] = pevent;
 
 	return 0;
 }
 
 static void op_destroy_counter(int cpu, int event)
 {
-	struct perf_event *pevent = perf_events[cpu][event];
+	struct perf_event *pevent = per_cpu(perf_events, cpu)[event];
 
 	if (pevent) {
 		perf_event_release_kernel(pevent);
-		perf_events[cpu][event] = NULL;
+		per_cpu(perf_events, cpu)[event] = NULL;
 	}
 }
 

@@ -257,12 +258,12 @@ void oprofile_perf_exit(void)
 
 	for_each_possible_cpu(cpu) {
 		for (id = 0; id < num_counters; ++id) {
-			event = perf_events[cpu][id];
+			event = per_cpu(perf_events, cpu)[id];
 			if (event)
 				perf_event_release_kernel(event);
 		}
 
-		kfree(perf_events[cpu]);
+		kfree(per_cpu(perf_events, cpu));
 	}
 
 	kfree(counter_config);

@@ -277,8 +278,6 @@ int __init oprofile_perf_init(struct oprofile_operations *ops)
 	if (ret)
 		return ret;
 
-	memset(&perf_events, 0, sizeof(perf_events));
-
 	num_counters = perf_num_counters();
 	if (num_counters <= 0) {
 		pr_info("oprofile: no performance counters\n");

@@ -298,9 +297,9 @@ int __init oprofile_perf_init(struct oprofile_operations *ops)
 	}
 
 	for_each_possible_cpu(cpu) {
-		perf_events[cpu] = kcalloc(num_counters,
+		per_cpu(perf_events, cpu) = kcalloc(num_counters,
 				sizeof(struct perf_event *), GFP_KERNEL);
-		if (!perf_events[cpu]) {
+		if (!per_cpu(perf_events, cpu)) {
 			pr_info("oprofile: failed to allocate %d perf events "
 					"for cpu %d\n", num_counters, cpu);
 			ret = -ENOMEM;