Re: [PATCH] powerpc/perfctr: Check oprofile_cpu_type for NULL before using it

2009-08-06 Thread David Woodhouse
On Thu, 2009-08-06 at 14:02 +1000, Benjamin Herrenschmidt wrote:
 If the current CPU doesn't support performance counters,
 cur_cpu_spec-oprofile_cpu_type can be NULL. The current
 perfctr modules don't test for that case and would thus
 crash.

It can't actually be NULL on a 64-bit CPU; all 64-bit CPUs in the table
have -oprofile_cpu_type set.

Of course, adding the check probably makes sense anyway.

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc/perfctr: Check oprofile_cpu_type for NULL before using it

2009-08-06 Thread Benjamin Herrenschmidt
On Thu, 2009-08-06 at 07:41 +0100, David Woodhouse wrote:
 On Thu, 2009-08-06 at 14:02 +1000, Benjamin Herrenschmidt wrote:
  If the current CPU doesn't support performance counters,
  cur_cpu_spec-oprofile_cpu_type can be NULL. The current
  perfctr modules don't test for that case and would thus
  crash.
 
 It can't actually be NULL on a 64-bit CPU; all 64-bit CPUs in the table
 have -oprofile_cpu_type set.

  today :-)

 Of course, adding the check probably makes sense anyway.

Yup, better safe.

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc/perfctr: Check oprofile_cpu_type for NULL before using it

2009-08-06 Thread Michael Ellerman
On Thu, 2009-08-06 at 14:02 +1000, Benjamin Herrenschmidt wrote:
 If the current CPU doesn't support performance counters,
 cur_cpu_spec-oprofile_cpu_type can be NULL. The current
 perfctr modules don't test for that case and would thus
 crash.

 + if (!cur_cpu_spec-oprofile_cpu_type ||
..
 + if (!cur_cpu_spec-oprofile_cpu_type ||
..
 + if (!cur_cpu_spec-oprofile_cpu_type ||
..
 + if (!cur_cpu_spec-oprofile_cpu_type ||
..
 + if (!cur_cpu_spec-oprofile_cpu_type ||
..
 + if (!cur_cpu_spec-oprofile_cpu_type ||
..
 + if (!cur_cpu_spec-oprofile_cpu_type ||

Typing it seven times didn't make you think how about a helper? :)

Perhaps:

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cput
index 80f315e..956cbc3 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -123,6 +123,11 @@ struct cpu_spec {
 
 extern struct cpu_spec *cur_cpu_spec;
 
+static inline int oprofile_cpu_type_matches(const char *s)
+{
+   return s  (strcmp(cur_cpu_spec-oprofile_cpu_type, s) == 0);
+}
+
 extern unsigned int __start___ftr_fixup, __stop___ftr_fixup;
 
 extern struct cpu_spec *identify_cpu(unsigned long offset, unsigned int pvr);


And then callsites become:

 static int init_mpc7450_pmu(void)
 {
if (!oprofile_cpu_type_matches(ppc/7450))
return -ENODEV;


cheers

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] powerpc/perfctr: Check oprofile_cpu_type for NULL before using it

2009-08-06 Thread Benjamin Herrenschmidt
On Thu, 2009-08-06 at 21:03 +1000, Michael Ellerman wrote:
 
 Typing it seven times didn't make you think how about a helper? :)

Copy/paste works fine :-) Seriously, I did think about it, but a couple
of cases test more than one string so a totally trivial helper wouldn't
do and I couldn't be bothered doing anything more complicated while
under influenza :-) You are welcome to do something nicer for .32.

BTW. While at it, I think we should move that stuff to a separate
subdir too.

Cheers,
Ben.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev