Re: [RFC] [PATCH] x86: avoid per_cpu for APIC id tables

[Ingo Molnar <mingo@kernel.org>]

* Ingo Molnar <mingo@kernel.org> wrote:

> [...]
> 
> Also, if the goal is to pack better then we could do even better than 
> that: we could create a 'struct x86_apic_ids':
> 
>   struct x86_apic_ids {
> 	u16 bios_apicid;
> 	u16 apicid;
> 	u32 logical_apicid;	/* NOTE: does this really have to be 32-bit? */
>   };
> 
> and put that into an explicit, [NR_CPUS] array. This preserves the tight 
> coupling between fields that PER_CPU offered, requiring only a single 
> cacheline fetch in the cache-cold case, while also giving efficient, 
> packed caching for cache-hot remote wakeups.
> 
> [ Assuming remote wakeups access all of these fields in the hot path to 
>   generate an IPI. Do they? ]
> 
> Also, this NR_CPUS array should be cache-aligned and read-mostly, to avoid 
> false sharing artifacts. Your current patch does not do either.

Btw., if you implement the changes I suggested and the patch still 
provides a robust 10% improvement in the cross-wakeup benchmark over the 
vanilla kernel then that will be a pretty good indication that it's the 
cache-hot layout and decreased indirection cost that makes the difference 
- and then we'd of course want to merge your patch upstream.

Also, a comment should be added to the new [NR_CPUS] array explaining that 
it's a special data structure that is almost always accessed from remote 
CPUs, and that for that reason PER_CPU accesses are sub-optimal: to 
prevent someone else from naively PER_CPU-ifying the [NR_CPUS] array later 
on ;-)

Thanks,

	Ingo