Skip to contentSkip to conversation

Fix perfctr oops on ppc32

PATCH In review David Woodhouse 2 participants 4 messages Aug 1, 2009 – Aug 5, 2009 PowerPC

lore
Share & tools
lore

David Woodhouse

author
#0 lore raw

PATCH 1/1 Fix perfctr oops on ppc32

This seems to be the reason why the Fedora rawhide 2.6.31-rc kernel doesn't boot. With some CPUs, cur_cpu_spec->oprofile_cpu_type can be NULL -- which makes strcmp() unhappy.

Signed-off-by David Woodhouse <[email protected]>

---
At first glance, it looks like there are a bunch of other places which use cur_cpu_spec->oprofile_cpu_type without first checking if it's non-NULL, but maybe those are all 64-bit and all 64-bit cpu types have it set?

arch/powerpc/kernel/mpc7450-pmu.c +2 -1
index 75ff47f..ea383c1 100644
@@ -408,7 +408,8 @@ struct power_pmu mpc7450_pmu = {
 
 static int init_mpc7450_pmu(void)
 {
-	if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
+	if (cur_cpu_spec->oprofile_cpu_type &&
+	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
 		return -ENODEV;
 
 	return register_power_pmu(&mpc7450_pmu);
Signature · 2 lines
David Woodhouse                            Open Source Technology Centre
[email protected]                              Intel Corporation
Acked-by on v1 1/1
#1 lore raw
Quoted 5 lines · David Woodhouse

This seems to be the reason why the Fedora rawhide 2.6.31-rc kernel doesn't boot. With some CPUs, cur_cpu_spec->oprofile_cpu_type can be NULL -- which makes strcmp() unhappy.

Signed-off-by David Woodhouse <[email protected]>

Thanks David !

Acked-by Benjamin Herrenschmidt <[email protected]>

---

Paulus, do you want to put that in your new perfctr-powerpc tree or I'll just stick that in my merge brange next week along with other fixes ?

Cheers,
Ben.

Quoted 22 lines

---
At first glance, it looks like there are a bunch of other places which use cur_cpu_spec->oprofile_cpu_type without first checking if it's non-NULL, but maybe those are all 64-bit and all 64-bit cpu types have it set?

arch/powerpc/kernel/mpc7450-pmu.c +2 -1
index 75ff47f..ea383c1 100644
@@ -408,7 +408,8 @@ struct power_pmu mpc7450_pmu = {
 
 static int init_mpc7450_pmu(void)
 {
-	if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
+	if (cur_cpu_spec->oprofile_cpu_type &&
+	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
 		return -ENODEV;
 
 	return register_power_pmu(&mpc7450_pmu);
#2 lore raw
In reply to David Woodhouse
Quoted 10 lines · David Woodhouse

This seems to be the reason why the Fedora rawhide 2.6.31-rc kernel doesn't boot. With some CPUs, cur_cpu_spec->oprofile_cpu_type can be NULL -- which makes strcmp() unhappy.

Signed-off-by David Woodhouse <[email protected]>

---
At first glance, it looks like there are a bunch of other places which use cur_cpu_spec->oprofile_cpu_type without first checking if it's non-NULL, but maybe those are all 64-bit and all 64-bit cpu types have it set?

Argh, ignore my Acked-by, I think the patch isn't right...

arch/powerpc/kernel/mpc7450-pmu.c struct power_pmu mpc7450_pmu = { · line 413

Quoted 12 lines
arch/powerpc/kernel/mpc7450-pmu.c +2 -1
index 75ff47f..ea383c1 100644
@@ -408,7 +408,8 @@ struct power_pmu mpc7450_pmu = {
 
 static int init_mpc7450_pmu(void)
 {
-	if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
+	if (cur_cpu_spec->oprofile_cpu_type &&
+	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
 		return -ENODEV;

That means that if we have oprofile_cpu_type, we will enable the 7450 PMCs which doesn't sound right. Shouldn't it be instead:

	if (!cur_cpu_spec->oprofile_cpu_type ||
	    strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))

return -ENODEV;

Cheers,
Ben.

Quoted 3 lines
arch/powerpc/kernel/mpc7450-pmu.c +0 -0
 	return register_power_pmu(&mpc7450_pmu);
author
#3 lore raw
Quoted 2 lines · Benjamin Herrenschmidt

Argh, ignore my Acked-by, I think the patch isn't right...

Hm, good point. Doh.

Signature · 2 lines
David Woodhouse                            Open Source Technology Centre
[email protected]                              Intel Corporation