Skip to contentSkip to conversation

drivers: CCI: fix used_mask init in validate_group()

PATCH Superseded Mark Salter 5 participants 20 messages Apr 8, 2015 – Apr 30, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers: CCI: fix used_mask init in validate_group()

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask. This patch changes the used_mask initialization to be runtime based on the actual size of the array.

---
 drivers/bus/arm-cci.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/bus/arm-cci.c +7 -7
index 84fd660..1d83072 100644
@@ -679,13 +679,13 @@ static int
 validate_group(struct perf_event *event)
 {
 	struct perf_event *sibling, *leader = event->group_leader;
-	struct cci_pmu_hw_events fake_pmu = {
-		/*
-		 * Initialise the fake PMU. We only need to populate the
-		 * used_mask for the purposes of validation.
-		 */
-		.used_mask = CPU_BITS_NONE,
-	};
+	struct cci_pmu_hw_events fake_pmu;
+
+	/*
+	 * Initialise the fake PMU. We only need to populate the
+	 * used_mask for the purposes of validation.
+	 */
+	memset(fake_pmu.used_mask, 0, sizeof(fake_pmu.used_mask));
 
 	if (!validate_event(&fake_pmu, leader))
 		return -EINVAL;
Signature · 1 line
1.9.3
Reviewed-by on v1 1/1
#1 lore raw
Quoted 26 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask. This patch changes the used_mask initialization to be runtime based on the actual size of the array.

---
  drivers/bus/arm-cci.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)
drivers/bus/arm-cci.c +0 -4
index 84fd660..1d83072 100644
@@ -679,13 +679,13 @@ static int
  validate_group(struct perf_event *event)
  {
  	struct perf_event *sibling, *leader = event->group_leader;
-	struct cci_pmu_hw_events fake_pmu = {
-		/*
-		 * Initialise the fake PMU. We only need to populate the
-		 * used_mask for the purposes of validation.

drivers/bus/arm-cci.c static int · line 688

Quoted 10 lines
drivers/bus/arm-cci.c +7 -3
-		 */
-		.used_mask = CPU_BITS_NONE,
-	};
+	struct cci_pmu_hw_events fake_pmu;
+
+	/*
+	 * Initialise the fake PMU. We only need to populate the
+	 * used_mask for the purposes of validation.
+	 */
+	memset(fake_pmu.used_mask, 0, sizeof(fake_pmu.used_mask));

The patch looks good to me.

Suzuki

#2 lore raw
In reply to Mark Salter
Quoted 6 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

Whoops. My bad.

Quoted 13 lines

This patch changes the used_mask initialization to be runtime based on the actual size of the array.

---
 drivers/bus/arm-cci.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/bus/arm-cci.c +0 -0
index 84fd660..1d83072 100644

drivers/bus/arm-cci.c static int · line 687

Quoted 9 lines
drivers/bus/arm-cci.c +0 -6
@@ -679,13 +679,13 @@ static int
 validate_group(struct perf_event *event)
 {
 	struct perf_event *sibling, *leader = event->group_leader;
-	struct cci_pmu_hw_events fake_pmu = {
-		/*
-		 * Initialise the fake PMU. We only need to populate the
-		 * used_mask for the purposes of validation.
-		 */
-		.used_mask = CPU_BITS_NONE,

Can we not simply change this to:

		.used_mask = { 0 },

That should result in the entire array being zeroed.

Thanks,
Mark.

#3 lore raw
Quoted 35 lines · Mark Rutland
Quoted 6 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

Whoops. My bad.

Quoted 13 lines

This patch changes the used_mask initialization to be runtime based on the actual size of the array.

---
 drivers/bus/arm-cci.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/bus/arm-cci.c +0 -0
index 84fd660..1d83072 100644

drivers/bus/arm-cci.c static int · line 687

Quoted 9 lines
drivers/bus/arm-cci.c +0 -6
@@ -679,13 +679,13 @@ static int
 validate_group(struct perf_event *event)
 {
 	struct perf_event *sibling, *leader = event->group_leader;
-	struct cci_pmu_hw_events fake_pmu = {
-		/*
-		 * Initialise the fake PMU. We only need to populate the
-		 * used_mask for the purposes of validation.
-		 */
-		.used_mask = CPU_BITS_NONE,

Can we not simply change this to:

Quoted 3 lines
		.used_mask = { 0 },

That should result in the entire array being zeroed.

It does, but it also causes the whole struct to be cleared. With the memset, only used_mask gets cleared.

#4 lore raw
Quoted 36 lines · Mark Salter
Quoted 35 lines · Mark Rutland
Quoted 6 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

Whoops. My bad.

Quoted 13 lines

This patch changes the used_mask initialization to be runtime based on the actual size of the array.

---
 drivers/bus/arm-cci.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/bus/arm-cci.c +0 -0
index 84fd660..1d83072 100644

drivers/bus/arm-cci.c static int · line 687

Quoted 9 lines
drivers/bus/arm-cci.c +0 -6
@@ -679,13 +679,13 @@ static int
 validate_group(struct perf_event *event)
 {
 	struct perf_event *sibling, *leader = event->group_leader;
-	struct cci_pmu_hw_events fake_pmu = {
-		/*
-		 * Initialise the fake PMU. We only need to populate the
-		 * used_mask for the purposes of validation.
-		 */
-		.used_mask = CPU_BITS_NONE,

Can we not simply change this to:

Quoted 5 lines
Quoted 3 lines
		.used_mask = { 0 },

That should result in the entire array being zeroed.

It does, but it also causes the whole struct to be cleared.

Sure, but it's also the minimal diff, and it's easier to read. This was what the code was intended to be initially.

Quoted 1 line

With the memset, only used_mask gets cleared.

Is there an appreciable difference between the two performance-wise?

Mark.

#5 lore raw
Quoted 41 lines · Mark Rutland
Quoted 40 lines · Mark Salter
Quoted 38 lines · Mark Rutland
Quoted 6 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

Whoops. My bad.

Quoted 13 lines

This patch changes the used_mask initialization to be runtime based on the actual size of the array.

---
 drivers/bus/arm-cci.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/bus/arm-cci.c +0 -0
index 84fd660..1d83072 100644

drivers/bus/arm-cci.c static int · line 687

Quoted 9 lines
drivers/bus/arm-cci.c +0 -6
@@ -679,13 +679,13 @@ static int
 validate_group(struct perf_event *event)
 {
 	struct perf_event *sibling, *leader = event->group_leader;
-	struct cci_pmu_hw_events fake_pmu = {
-		/*
-		 * Initialise the fake PMU. We only need to populate the
-		 * used_mask for the purposes of validation.
-		 */
-		.used_mask = CPU_BITS_NONE,

Can we not simply change this to:

		.used_mask = { 0 },

That should result in the entire array being zeroed.

Quoted 8 lines
Quoted 1 line

It does, but it also causes the whole struct to be cleared.

Sure, but it's also the minimal diff, and it's easier to read. This was what the code was intended to be initially.

Quoted 1 line

With the memset, only used_mask gets cleared.

Is there an appreciable difference between the two performance-wise?

I dunno. It is 3 strp insns vs 1 str.
If you want the static init, I'll send another patch.

#6 lore raw
Quoted 21 lines
Quoted 21 lines
Quoted 12 lines
Quoted 12 lines

drivers/bus/arm-cci.c static int · line 687

Quoted 9 lines

validate_group(struct perf_event *event)
{

drivers/bus/arm-cci.c +0 -6
 	struct perf_event *sibling, *leader = event->group_leader;
-	struct cci_pmu_hw_events fake_pmu = {
-		/*
-		 * Initialise the fake PMU. We only need to populate the
-		 * used_mask for the purposes of validation.
-		 */
-		.used_mask = CPU_BITS_NONE,

Can we not simply change this to:

Quoted 5 lines
Quoted 3 lines
		.used_mask = { 0 },

That should result in the entire array being zeroed.

It does, but it also causes the whole struct to be cleared.

Sure, but it's also the minimal diff, and it's easier to read. This was what the code was intended to be initially.

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

With the memset, only used_mask gets cleared.

Is there an appreciable difference between the two performance-wise?

I dunno. It is 3 strp insns vs 1 str.
If you want the static init, I'll send another patch.

I'd prefer the designated initializer to the memset.

Thanks,
Mark.

#7 lore raw

PATCH v2 1/1 drivers: CCI: fix used_mask init in validate_group()

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

---
 drivers/bus/arm-cci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/arm-cci.c +1 -1
index 84fd660..759e41c 100644
@@ -684,7 +684,7 @@ validate_group(struct perf_event *event)
 		 * Initialise the fake PMU. We only need to populate the
 		 * used_mask for the purposes of validation.
 		 */
-		.used_mask = CPU_BITS_NONE,
+		.used_mask = { 0 },
 	};
 
 	if (!validate_event(&fake_pmu, leader))
Signature · 1 line
1.8.3.1
Reviewed-by on v2 1/1
#8 lore raw
Quoted 8 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

Arnd, Olof, are you happy to take this via arm-soc?

My (broken) patch went via Will's tree because of a perf dependency, but other CCI patches have gone via you guys.

Thanks,
Mark.

Quoted 20 lines
---
 drivers/bus/arm-cci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/arm-cci.c +1 -1
index 84fd660..759e41c 100644
@@ -684,7 +684,7 @@ validate_group(struct perf_event *event)
 		 * Initialise the fake PMU. We only need to populate the
 		 * used_mask for the purposes of validation.
 		 */
-		.used_mask = CPU_BITS_NONE,
+		.used_mask = { 0 },
 	};
 
 	if (!validate_event(&fake_pmu, leader))
Signature · 1 line
1.8.3.1
#9 lore raw
Quoted 8 lines · Mark Rutland
Quoted 7 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

Quoted 8 lines
Quoted 1 line

Arnd, Olof, are you happy to take this via arm-soc?

My (broken) patch went via Will's tree because of a perf dependency, but other CCI patches have gone via you guys.

Yeah, arm-soc is the best place for this. You should resend to arm at kernel.org as a new patch with the relevant acks.

Will

#10 lore raw
Quoted 13 lines · Will Deacon
Quoted 12 lines · Mark Rutland
Quoted 8 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

Quoted 7 lines
Quoted 4 lines

Arnd, Olof, are you happy to take this via arm-soc?

My (broken) patch went via Will's tree because of a perf dependency, but other CCI patches have gone via you guys.

Yeah, arm-soc is the best place for this. You should resend to arm at kernel.org as a new patch with the relevant acks.

Mark

Could you please send this to arm-soc as suggested by Will, with the relevant acks/reviews ?

Thanks
Suzuki

#11 lore raw
Quoted 19 lines · Suzuki K. Poulose
Quoted 18 lines · Will Deacon
Quoted 16 lines · Mark Rutland
Quoted 8 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

Arnd, Olof, are you happy to take this via arm-soc?

My (broken) patch went via Will's tree because of a perf dependency, but other CCI patches have gone via you guys.

Quoted 7 lines
Quoted 2 lines

Yeah, arm-soc is the best place for this. You should resend to arm at kernel.org as a new patch with the relevant acks.

Mark

Could you please send this to arm-soc as suggested by Will, with the relevant acks/reviews ?

I sent it on Tuesday. Did it not show up? Is arm at kernel.org the correct address? I got the cc:

From: Mark Salter <[email protected]>
To: arm at kernel.org
Cc: Mark Salter <[email protected]>
Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
Date: Tue, 28 Apr 2015 13:09:32 -0400
Message-Id: <[email protected]>

#12 lore raw
Quoted 7 lines
Quoted 3 lines

Could you please send this to arm-soc as suggested by Will, with the relevant acks/reviews ?

I sent it on Tuesday. Did it not show up? Is arm at kernel.org the correct address? I got the cc:

Quoted 6 lines

From: Mark Salter <[email protected]>
To: arm at kernel.org
Cc: Mark Salter <[email protected]>
Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
Date: Tue, 28 Apr 2015 13:09:32 -0400
Message-Id: <[email protected]>

That's the right address, but that only goes to the maintainers, and doesn't get copied to any list. In future, please Cc linux-arm-kernel in addition.

Thanks,
Mark.

#13 lore raw
Quoted 14 lines · Mark Rutland
Quoted 7 lines
Quoted 3 lines

Could you please send this to arm-soc as suggested by Will, with the relevant acks/reviews ?

I sent it on Tuesday. Did it not show up? Is arm at kernel.org the correct address? I got the cc:

Quoted 6 lines

From: Mark Salter <[email protected]>
To: arm at kernel.org
Cc: Mark Salter <[email protected]>
Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
Date: Tue, 28 Apr 2015 13:09:32 -0400
Message-Id: <[email protected]>

Quoted 3 lines

That's the right address, but that only goes to the maintainers, and doesn't get copied to any list. In future, please Cc linux-arm-kernel in addition.

That's where I sent it originally.

#14 lore raw
Quoted 15 lines · Mark Salter
Quoted 14 lines · Mark Rutland
Quoted 13 lines
Quoted 3 lines

Could you please send this to arm-soc as suggested by Will, with the relevant acks/reviews ?

I sent it on Tuesday. Did it not show up? Is arm at kernel.org the correct address? I got the cc:

From: Mark Salter <[email protected]>
To: arm at kernel.org
Cc: Mark Salter <[email protected]>
Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
Date: Tue, 28 Apr 2015 13:09:32 -0400
Message-Id: <[email protected]>

Quoted 5 lines
Quoted 3 lines

That's the right address, but that only goes to the maintainers, and doesn't get copied to any list. In future, please Cc linux-arm-kernel in addition.

That's where I sent it originally.

Sure, but it's good to Cc when sending to arm-soc so as to make it visible that the patches have been sent. Doing so avoids the necessity of queries like Suzuki's, and makes it possible for others to reply to the version sent to arm at kernel.org in the case of conflicts or other issues.

Thanks,
Mark.

#15 lore raw
Quoted 20 lines · Mark Rutland
Quoted 19 lines · Mark Salter
Quoted 17 lines · Mark Rutland
Quoted 7 lines
Quoted 3 lines

Could you please send this to arm-soc as suggested by Will, with the relevant acks/reviews ?

I sent it on Tuesday. Did it not show up? Is arm at kernel.org the correct address? I got the cc:

Quoted 6 lines

From: Mark Salter <[email protected]>
To: arm at kernel.org
Cc: Mark Salter <[email protected]>
Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
Date: Tue, 28 Apr 2015 13:09:32 -0400
Message-Id: <[email protected]>

That's the right address, but that only goes to the maintainers, and doesn't get copied to any list. In future, please Cc linux-arm-kernel in addition.

Quoted 7 lines
Quoted 1 line

That's where I sent it originally.

Sure, but it's good to Cc when sending to arm-soc so as to make it visible that the patches have been sent. Doing so avoids the necessity of queries like Suzuki's, and makes it possible for others to reply to the version sent to arm at kernel.org in the case of conflicts or other issues.

But why did it need to be sent to a private maintainer's list in the first place? I think that the destination addresses of the original posting was perfectly reasonable given output from get_maintainer.pl and that sending me to a private list was an unnecessary hoop to jump through.

#16 lore raw
Quoted 29 lines · On Thursday 30 April 2015 10:46:13 Mark Salter wrote:
Quoted 20 lines · Mark Rutland
Quoted 19 lines · Mark Salter
Quoted 17 lines · Mark Rutland
Quoted 7 lines
Quoted 3 lines

Could you please send this to arm-soc as suggested by Will, with the relevant acks/reviews ?

I sent it on Tuesday. Did it not show up? Is arm at kernel.org the correct address? I got the cc:

Quoted 6 lines

From: Mark Salter <[email protected]>
To: arm at kernel.org
Cc: Mark Salter <[email protected]>
Subject: [PATCH V2] drivers: CCI: fix used_mask init in validate_group()
Date: Tue, 28 Apr 2015 13:09:32 -0400
Message-Id: <[email protected]>

That's the right address, but that only goes to the maintainers, and doesn't get copied to any list. In future, please Cc linux-arm-kernel in addition.

Quoted 7 lines
Quoted 1 line

That's where I sent it originally.

Sure, but it's good to Cc when sending to arm-soc so as to make it visible that the patches have been sent. Doing so avoids the necessity of queries like Suzuki's, and makes it possible for others to reply to the version sent to arm at kernel.org in the case of conflicts or other issues.

Quoted 5 lines

But why did it need to be sent to a private maintainer's list in the first place? I think that the destination addresses of the original posting was perfectly reasonable given output from get_maintainer.pl and that sending me to a private list was an unnecessary hoop to jump through.

The purpose of the arm at kernel.org alias is for subarch maintainers to send us stuff, it's not really meant for normal developers, unless specifically advised by a maintainer. Each file we maintain through arm-soc normally belongs to one subarch, so we tend to not pick up any patches on the mailing list and instead wait for that subarch maintainer to pick them up and forward the changes to us.

That model model breaks down to some degree for drivers/bus, in particular for stuff that is not specific to just one SoC. I have the patch in my todo list now, sorry about missing that earlier. We should probably come up with a better way to handle patches like this one.

Arnd

#17 lore raw
In reply to Mark Salter
Quoted 18 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

---
  drivers/bus/arm-cci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/arm-cci.c +0 -0
index 84fd660..759e41c 100644

drivers/bus/arm-cci.c validate_group(struct perf_event *event) · line 690

Quoted 8 lines
drivers/bus/arm-cci.c +1 -1
@@ -684,7 +684,7 @@ validate_group(struct perf_event *event)
  		 * Initialise the fake PMU. We only need to populate the
  		 * used_mask for the purposes of validation.
  		 */
-		.used_mask = CPU_BITS_NONE,
+		.used_mask = { 0 },
  	};
 
  	if (!validate_event(&fake_pmu, leader))

I have a series of patches to add CCI-500 PMU, targeting 4.2, which changes the used_mask to a pointer and changes it to depend on the number of counters available on the CCI PMU. I will post it in very soon here, which could make this patch obsolete. Let me know if you don't mind getting this sorted out through my series. I am fine either way.

Regards
Suzuki

#18 lore raw
Quoted 25 lines · Suzuki K. Poulose
Quoted 24 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

---
  drivers/bus/arm-cci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/arm-cci.c +1 -1
index 84fd660..759e41c 100644
@@ -684,7 +684,7 @@ validate_group(struct perf_event *event)
  		 * Initialise the fake PMU. We only need to populate the
  		 * used_mask for the purposes of validation.
  		 */
-		.used_mask = CPU_BITS_NONE,
+		.used_mask = { 0 },
  	};
 
Quoted 7 lines

drivers/bus/arm-cci.c validate_group(struct perf_event *event) · line 690

Quoted 2 lines
drivers/bus/arm-cci.c +0 -0
  	if (!validate_event(&fake_pmu, leader))

I have a series of patches to add CCI-500 PMU, targeting 4.2, which changes the used_mask to a pointer and changes it to depend on the number of counters available on the CCI PMU. I will post it in very soon here, which could make this patch obsolete. Let me know if you don't mind getting this sorted out through my series. I am fine either way.

I'd rather have the fix in for 4.1 and then have your CCI-500 patches based on that.

Will

#19 lore raw
Quoted 34 lines · Will Deacon
Quoted 25 lines · Suzuki K. Poulose
Quoted 24 lines · Mark Salter

Currently in validate_group(), there is a static initializer for fake_pmu.used_mask which is based on CPU_BITS_NONE but the used_mask array size is based on CCI_PMU_MAX_HW_EVENTS. CCI_PMU_MAX_HW_EVENTS is not based on NR_CPUS, so CPU_BITS_NONE is not correct and will cause a build failure if NR_CPUS is set high enough to make CPU_BITS_NONE larger than used_mask.

---
   drivers/bus/arm-cci.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bus/arm-cci.c +1 -1
index 84fd660..759e41c 100644
@@ -684,7 +684,7 @@ validate_group(struct perf_event *event)
   		 * Initialise the fake PMU. We only need to populate the
   		 * used_mask for the purposes of validation.
   		 */
-		.used_mask = CPU_BITS_NONE,
+		.used_mask = { 0 },
   	};
 
Quoted 7 lines

drivers/bus/arm-cci.c validate_group(struct perf_event *event) · line 690

Quoted 2 lines
drivers/bus/arm-cci.c +0 -0
   	if (!validate_event(&fake_pmu, leader))

I have a series of patches to add CCI-500 PMU, targeting 4.2, which changes the used_mask to a pointer and changes it to depend on the number of counters available on the CCI PMU. I will post it in very soon here, which could make this patch obsolete. Let me know if you don't mind getting this sorted out through my series. I am fine either way.

Quoted 5 lines

I'd rather have the fix in for 4.1 and then have your CCI-500 patches based on that.

Will

OK, will rebase my patches on top of this one.

Thanks
Suzuki