Re: [PATCH] x86: eliminate redundant/contradicting cache line size config options

Nick Piggin
2009-11-16T04:14:05Z

On Fri, Nov 13, 2009 at 11:54:40AM +0000, Jan Beulich wrote:
> Rather than having X86_L1_CACHE_BYTES and X86_L1_CACHE_SHIFT (with
> inconsistent defaults), just having the latter suffices as the former
> can be easily calculated from it.
>
> To be consistent, also change X86_INTERNODE_CACHE_BYTES to
> X86_INTERNODE_CACHE_SHIFT, and set it to 7 (128 bytes) for NUMA to
> account for last level cache line size (which here matters more than
> L1 cache line size).

I think if we're going to set it to 7 (128B, for Pentium 4), then
we should set the L1 cache shift as well? Most alignments to
prevent cacheline pingpong use L1 cache shift for this anyway?

The internode thing is really just a not quite well defined thing
because internode cachelines are really expensive and really big
on vsmp so they warrant trading off extra space on some critical
structures to reduce pingpong (but this is not to say that other
structures that are *not* internode annotated do *not* need to
worry about pingpong).

Put another way, big SMP P4 systems with !NUMA still want to have
128 byte aligned variables. I think it should just default to
L1_CACHE_SHIFT unless the special case of vsmp.

But other than that complaint (which was not introduced by your
patch anyway) then this looks like a good cleanup to me, thanks.

> Finally, make sure the default value for X86_L1_CACHE_SHIFT, when
> X86_GENERIC is selected, is being seen before that for the individual
> CPU model options (other than on x86-64, where GENERIC_CPU is part of
> the choice construct, X86_GENERIC is a separate option on ix86).
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> Cc: Nick Piggin <npiggin@suse.de>