Re: [PATCH UPDATED#2 04/16] x86: Use local variable to cache smp_processor_id() in setup_local_APIC()

[Tejun Heo <tj@kernel.org>]

On 11/30/2010 07:22 PM, Cyrill Gorcunov wrote:
> On Tue, Nov 30, 2010 at 07:11:45PM +0100, Tejun Heo wrote:
>> This is a trivial clean up.
>>
>> * Move initialization of @cpu inside preemption disabled region as
>>   suggested by Cyrill Gorcunov.
>>
>> Signed-off-by: Tejun Heo <tj@kernel.org>
>> Cc: Yinghai Lu <yinghai@kernel.org>
>> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
>> ---
>> Alright, updated yet again, but I frankly don't see the point in these
> 
> code structure, if we would be referring variable taken inside preempt
> off this always look suspicious, i think you agree with me :)

Not really.  smp_processor_id() outside of preemption/irq disabled
area is suspicious exactly the same and having @cpu initialized at the
head of the function has the advantage of making it clear that the
function as a whole is special w.r.t. which cpu it runs on.  In fact,
I think it's actually worse to put it inside preemption disabled area
as it gives the wrong impression that the function isn't special and
might be able to handle being called from another cpu.  In fact,
caching smp_processor_id() in @cpu is better than calling it every
time as it makes it clear that the function is bound to that cpu.

That said, I really don't think this matters one way or the other.
This function is called during smp bring up to initialize the CPU.
Things don't get much more arch/cpu dependent than this and the code
is thoroughly tested on each and every boot and power management
operations.  How @cpu is cached and used just doesn't matter, which is
why I didn't really care about it and changed as suggested.  So, let's
just let it go.

Thanks.

-- 
tejun