From: Nick Piggin <npiggin@suse.de>
To: Jan Beulich <JBeulich@novell.com>
Cc: mingo@elte.hu, tglx@linutronix.de, hpa@zytor.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86: eliminate redundant/contradicting cache line size config options
Date: Mon, 16 Nov 2009 05:14:07 +0100 [thread overview]
Message-ID: <20091116041407.GB5818@wotan.suse.de> (raw)
In-Reply-To: <4AFD5710020000780001F8F0@vpn.id2.novell.com>
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>
next prev parent reply other threads:[~2009-11-16 4:14 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-11-13 11:54 Jan Beulich
2009-11-16 4:14 ` Nick Piggin [this message]
2009-11-16 8:08 ` Jan Beulich
2009-11-16 10:56 ` Nick Piggin
2009-11-19 3:56 ` Ingo Molnar
2009-11-19 4:52 ` Arjan van de Ven
2009-11-19 8:13 ` Nick Piggin
2009-11-19 8:38 ` Jan Beulich
2009-11-19 10:00 ` Nick Piggin
2009-11-19 15:59 ` Arjan van de Ven
2009-11-19 16:18 ` Nick Piggin
2009-11-19 17:53 ` Andi Kleen
2009-11-23 8:34 ` Ingo Molnar
2009-11-23 9:35 ` Nick Piggin
2009-11-23 10:09 ` Ingo Molnar
2009-11-23 14:52 ` Arjan van de Ven
2009-11-23 15:15 ` Nick Piggin
2009-11-19 4:42 ` [tip:x86/mm] x86: Eliminate " tip-bot for Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20091116041407.GB5818@wotan.suse.de \
--to=npiggin@suse.de \
--cc=JBeulich@novell.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®