MIPS: [raw_]smp_processor_id uses current_thread_info

MIPS: [raw_]smp_processor_id uses current_thread_info

[Randy MacLeod]

Hi,

I'd like advice on changing the implementation of smp_processor_id on
Cavium specifically and/or MIPS generally.

Currently we have: arch/mips/include/asm/smp.h
#define raw_smp_processor_id() (current_thread_info()->cpu)

A co-worker has an issue where the current thread pointer is corrupted
on a Cavium MIPS system running 2.6.14 (but the same code exists in 2.6.31).
During the resulting panic() the kernel calls smp_processor_id()
which dereferences the corrupt task pointer again - ouch. I've notice that
other arches have raw_smp_processor_id() defined to
 - a platform specific register read, or
 - a percpu variable or
 - have a hard_smp_processor_id() defined
This last one is presumably for times when you don't trust the kernel
data structures to be
sane.

I can create a patch that calls cvmx_get_core_num(); for cavium.
Is there a more generic way to get the cpu number on MIPS?

Thanks,
-- 
../Randy/..

Re: MIPS: [raw_]smp_processor_id uses current_thread_info

[Ralf Baechle]

On Wed, Sep 23, 2009 at 02:27:26PM -0400, Randy MacLeod wrote:

> I'd like advice on changing the implementation of smp_processor_id on
> Cavium specifically and/or MIPS generally.
> 
> Currently we have: arch/mips/include/asm/smp.h
> #define raw_smp_processor_id() (current_thread_info()->cpu)
> 
> A co-worker has an issue where the current thread pointer is corrupted
> on a Cavium MIPS system running 2.6.14 (but the same code exists in 2.6.31).
> During the resulting panic() the kernel calls smp_processor_id()
> which dereferences the corrupt task pointer again - ouch. I've notice that
> other arches have raw_smp_processor_id() defined to
>  - a platform specific register read, or
>  - a percpu variable or
>  - have a hard_smp_processor_id() defined
> This last one is presumably for times when you don't trust the kernel
> data structures to be
> sane.

Dereferencing current_thread_info()->cpu is fairly likely to hit in the cache
so probably a single cycle operation.  raw_smp_processor_id() is also a
very common operation so you really don't want to change it to something
slower except for a debugging kernel.

If you have a good kernel stack pointer you can compute the thread pointer
from that:

	ori     $28, sp, _THREAD_MASK
	xori    $28, _THREAD_MASK

> I can create a patch that calls cvmx_get_core_num(); for cavium.
> Is there a more generic way to get the cpu number on MIPS?

raw_smp_processor_id() returns the processor ID as counted by Linux.  That
number does not necessarily match the firmware's numbering.

  Ralf

Re: MIPS: [raw_]smp_processor_id uses current_thread_info

[Kevin D. Kissell]

[-- Attachment #1: Type: text/plain, Size: 2143 bytes --]

Ralf Baechle wrote:
> On Wed, Sep 23, 2009 at 02:27:26PM -0400, Randy MacLeod wrote:
>
>   
>> I'd like advice on changing the implementation of smp_processor_id on
>> Cavium specifically and/or MIPS generally.
>>
>> Currently we have: arch/mips/include/asm/smp.h
>> #define raw_smp_processor_id() (current_thread_info()->cpu)
>>
>> A co-worker has an issue where the current thread pointer is corrupted
>> on a Cavium MIPS system running 2.6.14 (but the same code exists in 2.6.31).
>> During the resulting panic() the kernel calls smp_processor_id()
>> which dereferences the corrupt task pointer again - ouch. I've notice that
>> other arches have raw_smp_processor_id() defined to
>>  - a platform specific register read, or
>>  - a percpu variable or
>>  - have a hard_smp_processor_id() defined
>> This last one is presumably for times when you don't trust the kernel
>> data structures to be
>> sane.
>>     
>
> Dereferencing current_thread_info()->cpu is fairly likely to hit in the cache
> so probably a single cycle operation.
Ironically, it's statistically faster than reading a dedicated CP0 
register on those cores that have them, even if that were otherwise a 
good idea (see below), since access to CP0 registers generally doesn't 
pipeline!

>   raw_smp_processor_id() is also a
> very common operation so you really don't want to change it to something
> slower except for a debugging kernel.
>
> If you have a good kernel stack pointer you can compute the thread pointer
> from that:
>
> 	ori     $28, sp, _THREAD_MASK
> 	xori    $28, _THREAD_MASK
>
>   
>> I can create a patch that calls cvmx_get_core_num(); for cavium.
>> Is there a more generic way to get the cpu number on MIPS?
>>     
>
> raw_smp_processor_id() returns the processor ID as counted by Linux.  That
> number does not necessarily match the firmware's numbering.
Nor does it necessarily match the MIPS32R2's hardware CPU number in the 
EBase register.  smp_processor_id() is fundamentally a software concept, 
and it's more a lucky coincidence than an ironclad rule when it tracks 
hardware/firmware numbering.


          Regards,

          Kevin K.

[-- Attachment #2: Type: text/html, Size: 2777 bytes --]