Re: [PATCH 4/5] x86-64: Remove the PDA

[Brian Gerst]

On Sun, Jan 18, 2009 at 9:18 PM, Tejun Heo <tj@xxxxxxxxxx> wrote:
> Hello, Brian.
>
> Brian Gerst wrote:
> ...
>> @@ -881,13 +881,9 @@ __setup("clearcpuid=", setup_disablecpuid);
>>  #ifdef CONFIG_X86_64
>>  struct desc_ptr idt_descr = { 256 * 16 - 1, (unsigned long) idt_table };
>>
>> -DEFINE_PER_CPU_PAGE_ALIGNED(char[IRQ_STACK_SIZE], irq_stack);
>> -#ifdef CONFIG_SMP
>> -DEFINE_PER_CPU(char *, irq_stack_ptr);       /* will be set during per cpu init */
>> -#else
>> +DEFINE_PER_CPU_FIRST(union irq_stack_union, irq_stack_union) __aligned(PAGE_SIZE);
>>  DEFINE_PER_CPU(char *, irq_stack_ptr) =
>> -     per_cpu_var(irq_stack) + IRQ_STACK_SIZE - 64;
>> -#endif
>> +     per_cpu_var(irq_stack_union.irq_stack) + IRQ_STACK_SIZE - 64;
>>
>>  DEFINE_PER_CPU(unsigned long, kernel_stack) =
>>       (unsigned long)&init_thread_union - KERNEL_STACK_OFFSET + THREAD_SIZE;
>>
>> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
>> index 98ea26a..8c83de6 100644
>> --- a/arch/x86/kernel/head_64.S
>> +++ b/arch/x86/kernel/head_64.S
>> @@ -216,6 +216,7 @@ ENTRY(secondary_startup_64)
>>       cmpl    $0, per_cpu__cpu_number(%rax)
>>       jne     1f
>>       addq    %rax, early_gdt_descr_base(%rip)
>> +     addq    %rax, per_cpu__irq_stack_ptr(%rax)
>>  1:
>>  #endif
>>       /*
>
> As discussed before, the above chunks do drop one #ifdef CONFIG_SMP
> but it does add a obscure relocation and please note that it's
> different from early_gdt_descr.  early_gdt_descr is needed to bring up
> the cpu so there's no other way to do it but to relocate it in
> assembly.  If you absolutely have to relocate irq_stack_ptr early,
> please do it in C code in head64.c but then again irq_stack_ptr is not
> even necessary till traps_init() which is way after per cpu area
> setup.  So, the above two chunks are not necessary && even if they go
> in, they don't have much to do with this patch.

I'll give you that this particular variable doesn't need early
adjustment currently.  I'd prefer if you left the ifdef off the first
hunk, though.  A comment will suffice to document that the initial
value is going to be overwritten later on SMP.

--
Brian Gerst
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/