Re: [PATCH] MIPS: oprofile: Fix for BUG: using smp_processor_id() in preemptible [00000000] code: oprofiled/1362

[Jerin Jacob <jerinjacobk@gmail.com>]

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@gmail.com>
> 
> 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;
>  	}