[RFC] MIPS: Prevent "BUG: using smp_processor_id() in preemptible..." errors

[RFC] MIPS: Prevent "BUG: using smp_processor_id() in preemptible..." errors

[Paul Martin]

Commit 9b26616c8d9dae53fbac7f7cb2c6dd1308102976 "MIPS: Respect the ISA
level in FCSR handling" added references to current_cpu_data, which is
a macro expanding to cpu_data[smp_processor_id()].  Change these to
raw_current_cpu_data.

These changes may or may not be a good idea.

There may also be a need for a similar change in
arch/mips/kernel/ptrace.c which I haven't made.

---
 arch/mips/include/asm/elf.h | 4 ++--
 arch/mips/math-emu/cp1emu.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index a594d8e..63f3bbc 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -304,7 +304,7 @@ do {									\
 									\
 	current->thread.abi = &mips_abi;				\
 									\
-	current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31;		\
+	current->thread.fpu.fcr31 = raw_current_cpu_data.fpu_csr31;	\
 } while (0)
 
 #endif /* CONFIG_32BIT */
@@ -366,7 +366,7 @@ do {									\
 	else								\
 		current->thread.abi = &mips_abi;			\
 									\
-	current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31;		\
+	current->thread.fpu.fcr31 = raw_current_cpu_data.fpu_csr31;	\
 									\
 	p = personality(current->personality);				\
 	if (p != PER_LINUX32 && p != PER_LINUX)				\
diff --git a/arch/mips/math-emu/cp1emu.c b/arch/mips/math-emu/cp1emu.c
index d31c537..52ea70c 100644
--- a/arch/mips/math-emu/cp1emu.c
+++ b/arch/mips/math-emu/cp1emu.c
@@ -921,7 +921,7 @@ static inline void cop1_ctc(struct pt_regs *xcp, struct mips_fpu_struct *ctx,
 			 (void *)xcp->cp0_epc, MIPSInst_RT(ir), value);
 
 		/* Preserve read-only bits.  */
-		mask = current_cpu_data.fpu_msk31;
+		mask = raw_current_cpu_data.fpu_msk31;
 		fcr31 = (value & ~mask) | (fcr31 & mask);
 		break;
 
-- 
2.1.4



-- 
Paul Martin                                  http://www.codethink.co.uk/
Senior Software Developer, Codethink Ltd.

Re: [RFC] MIPS: Prevent "BUG: using smp_processor_id() in preemptible..." errors

[Markos Chandras]

Hi,

On 04/22/2015 06:24 PM, Paul Martin wrote:
> Commit 9b26616c8d9dae53fbac7f7cb2c6dd1308102976 "MIPS: Respect the ISA
> level in FCSR handling" added references to current_cpu_data, which is
> a macro expanding to cpu_data[smp_processor_id()].  Change these to
> raw_current_cpu_data.
> 
> These changes may or may not be a good idea.
> 
> There may also be a need for a similar change in
> arch/mips/kernel/ptrace.c which I haven't made.
> 

(^^^ missed your SOB line here? But since this RFC that might have been
intentional)

> ---
>  arch/mips/include/asm/elf.h | 4 ++--
>  arch/mips/math-emu/cp1emu.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
> index a594d8e..63f3bbc 100644
> --- a/arch/mips/include/asm/elf.h
> +++ b/arch/mips/include/asm/elf.h
> @@ -304,7 +304,7 @@ do {									\
>  									\
>  	current->thread.abi = &mips_abi;				\
>  									\
> -	current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31;		\
> +	current->thread.fpu.fcr31 = raw_current_cpu_data.fpu_csr31;	\

The change looks sensible. I am wondering if
boot_cpu_data(==cpu_data[0]) is a better option though.

I leave that to Maciej.

-- 
markos

Re: [RFC] MIPS: Prevent "BUG: using smp_processor_id() in preemptible..." errors

[Markos Chandras]

Hi,

On 04/22/2015 06:24 PM, Paul Martin wrote:
> Commit 9b26616c8d9dae53fbac7f7cb2c6dd1308102976 "MIPS: Respect the ISA
> level in FCSR handling" added references to current_cpu_data, which is
> a macro expanding to cpu_data[smp_processor_id()].  Change these to
> raw_current_cpu_data.
> 
> These changes may or may not be a good idea.
> 
> There may also be a need for a similar change in
> arch/mips/kernel/ptrace.c which I haven't made.
> 

(^^^ missed your SOB line here? But since this RFC that might have been
intentional)

> ---
>  arch/mips/include/asm/elf.h | 4 ++--
>  arch/mips/math-emu/cp1emu.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
> index a594d8e..63f3bbc 100644
> --- a/arch/mips/include/asm/elf.h
> +++ b/arch/mips/include/asm/elf.h
> @@ -304,7 +304,7 @@ do {									\
>  									\
>  	current->thread.abi = &mips_abi;				\
>  									\
> -	current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31;		\
> +	current->thread.fpu.fcr31 = raw_current_cpu_data.fpu_csr31;	\

The change looks sensible. I am wondering if
boot_cpu_data(==cpu_data[0]) is a better option though.

I leave that to Maciej.

-- 
markos

Re: [RFC] MIPS: Prevent "BUG: using smp_processor_id() in preemptible..." errors

[Maciej W. Rozycki]

On Thu, 23 Apr 2015, Markos Chandras wrote:

> > Commit 9b26616c8d9dae53fbac7f7cb2c6dd1308102976 "MIPS: Respect the ISA
> > level in FCSR handling" added references to current_cpu_data, which is
> > a macro expanding to cpu_data[smp_processor_id()].  Change these to
> > raw_current_cpu_data.
> > 
> > These changes may or may not be a good idea.

 Good catch, thanks!  I don't use the kernel preemption feature, so it 
slipped through my testing.

> > diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
> > index a594d8e..63f3bbc 100644
> > --- a/arch/mips/include/asm/elf.h
> > +++ b/arch/mips/include/asm/elf.h
> > @@ -304,7 +304,7 @@ do {									\
> >  									\
> >  	current->thread.abi = &mips_abi;				\
> >  									\
> > -	current->thread.fpu.fcr31 = current_cpu_data.fpu_csr31;		\
> > +	current->thread.fpu.fcr31 = raw_current_cpu_data.fpu_csr31;	\
> 
> The change looks sensible. I am wondering if
> boot_cpu_data(==cpu_data[0]) is a better option though.

 Yeah, we have this schizophrenic arrangement where we collect per-CPU 
features first according to what individual processors support and then 
implicitly assert (without really checking) that CPU[0] is the weakest and 
its settings will do for the rest.  So e.g. all `cpu_has_*' macros expand 
to `cpu_data[0].*' regardless of what CPU they're called from (why they 
ask for `cpu_data[0]' explictly rather that `boot_cpu_data' escapes me).

 I think we should reconsider it all and rework (simplify) one day, and 
maybe decide how much asymmetry we can afford and consequently have noted 
on a per-CPU basis, but for the time being let's have `boot_cpu_data' here 
as well.  Thanks!

  Maciej