Re: [PATCH 03/12] x86-64: Convert irqstacks to per-cpu

[Tejun Heo]

Hello, Brian.

Brian Gerst wrote:
>>    * irq_stack_ptr is not used till traps_init(), no reason to
>>      initialize it early.  On SMP, just leaving it NULL till proper
>>      initialization in setup_per_cpu_areas() works.  Dropped
>>      is_boot_cpu and early irq_stack_ptr initialization.
> 
> And it adds more #ifdefs.  All these conditional initializations on
> CONFIG_SMP are really cluttering up the code.

Yes, at the cost of removing a hidden relocation in head_64.S.  We can
also remove #ifdef there and just comment that it will be overridden
during setup_per_cpu_areas() but I think #ifdef there is better for
documentation purposes.

> It also conflicts with one of my goals with these patches: have as
> much state as possible already prepared when a cpu boots.  For the
> boot cpu this means static initialization.  For secondary cpus, that
> means setting up the values in setup_per_cpu_areas().  This
> eliminates any window where the state isn't ready yet, as you've
> already seen with per_cpu_offset.

I like the goal but it has certain dangers in the current form because
the boot cpu is using the init data area not its actual percpu area,
so each usage needs to be closely controlled till actual per cpu areas
are setup, so I think in the current form I don't think there's a good
reason to hurry initialization of variables which are not used early.
It also in a way fragments initialization paths further.  It would be
nice if boot cpu and secondary cpus can do it using the same path at
different times.

> And is_boot_cpu was a worthwhile optimization on its own.  I had plans
> on using it in more places later.

I didn't have any objection against it but with the additional
relocation removed, it looked out of place in the patch.  Please feel
free to add it back as necessary.

Thanks.

-- 
tejun
--
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/