On Thu, Aug 01, 2013 at 06:31:05PM +0200, Ralf Baechle wrote: > On Thu, Aug 01, 2013 at 08:40:41PM +0530, Jerin Jacob wrote: > > > current_cpu_type() is not preemption-safe. > > If CONFIG_PREEMPT is enabled then mipsxx_reg_setup() can be called from preemptible state. > > Added get_cpu()/put_cpu() pair to make it preemption-safe. > > > > This was found while testing oprofile with CONFIG_DEBUG_PREEMPT enable. > [...] > > Interesting. I wonder how many more of this kind of bug we got lurking > around. > > In case of oprofile, we silently assume that all processors have the same > number of counters, the same style and number of counters. So it'd be > ok to just look at the boot CPU which is even simpler than your fix. > > What do you think about below fix? yes, it make sense to compare the cpu_type against boot cpu. Acked-by: Jerin Jacob <jerinjacobk@xxxxxxxxx> > > Thanks, > > Ralf > > arch/mips/include/asm/cpu-features.h | 2 ++ > arch/mips/oprofile/op_model_mipsxx.c | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h > index 1dc0860..fa44f3e 100644 > --- a/arch/mips/include/asm/cpu-features.h > +++ b/arch/mips/include/asm/cpu-features.h > @@ -17,6 +17,8 @@ > #define current_cpu_type() current_cpu_data.cputype > #endif > > +#define boot_cpu_type() cpu_data[0].cputype > + > /* > * SMP assumption: Options of CPU 0 are a superset of all processors. > * This is true for all known MIPS systems. > diff --git a/arch/mips/oprofile/op_model_mipsxx.c b/arch/mips/oprofile/op_model_mipsxx.c > index e4b1140..3a2b6e9 100644 > --- a/arch/mips/oprofile/op_model_mipsxx.c > +++ b/arch/mips/oprofile/op_model_mipsxx.c > @@ -166,7 +166,7 @@ static void mipsxx_reg_setup(struct op_counter_config *ctr) > reg.control[i] |= M_PERFCTL_USER; > if (ctr[i].exl) > reg.control[i] |= M_PERFCTL_EXL; > - if (current_cpu_type() == CPU_XLR) > + if (boot_cpu_type() == CPU_XLR) > reg.control[i] |= M_PERFCTL_COUNT_ALL_THREADS; > reg.counter[i] = 0x80000000 - ctr[i].count; > }
