public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: Lin Ming <ming.m.lin@intel.com>, Ingo Molnar <mingo@elte.hu>,
	Andi Kleen <andi@firstfloor.org>,
	Stephane Eranian <eranian@google.com>,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/7] perf: Check if HT is supported and enabled
Date: Tue, 04 Jan 2011 12:10:19 +0100	[thread overview]
Message-ID: <1294139419.2016.115.camel@laptop> (raw)
In-Reply-To: <4D222926.9010206@zytor.com>

On Mon, 2011-01-03 at 11:53 -0800, H. Peter Anvin wrote:
> On 01/03/2011 02:58 AM, Peter Zijlstra wrote:
> >>
> >> This function can be simplified as below,
> >>
> >> static bool ht_enabled()
> >> {
> >>         if (!cpu_has(&boot_cpu_data, X86_FEATURE_HT))
> >>                 return false;
> >>
> >>         return smp_num_siblings > 1;
> >> }
> >>
> >> But this still can't detect if HT is on or off.
> >> smp_num_siblings is always 2 even if HT is disabled in BIOS.
> >>
> >> Any idea how to detect if HT is on or not?
> > 
> > Not quite sure, the intel docs aren't really clear on how the HW
> > supports HT, has 2 siblings but BIOS disabled it thing works. I just
> > tried reading the arch/x86 code but that only got me more confused.
> > 
> > hpa, could you comment?
> > 
> 
> Zeroeth of all: anyone who writes () in a function prototype in C needs
> to get severely napalmed, maimed, hung and then really hurt.  It is
> (void) in C, () means (...) which is literally NEVER what you want.
> 
> Now, *first* of all: smp_num_siblings as it sits is obviously broken;
> the whole notion of a global variable for what is inherently a per-cpu
> property is just braindead.  At least theoretically there could be cores
> with and without HT or with different thread count in the same system.
> 
> Second, perf should get this from /proc/cpuinfo, not by grotting around
> cpuid by itself.

Its the kernel space bit that wants to know if HT is enabled on CPU
bringup, /proc/cpuinfo is kinda useless in that context.

> Third, the code in the kernel is indeed pretty confusing, and it also
> has the global variable braindamage... but either it works (in which
> case getting the data from /proc/cpuinfo works) or it needs fixing in
> addition to the global variable issue.

So you failed to address the primary question, how do we tell if HT is
enabled for a particular CPU?

X86_FEATURE_HT tells us the CPU supports telling us about HT,
smp_num_siblings > 1 tells us the CPU is capable of HT. But Lin found
that if you disable HT in the BIOS both those are true but we still
don't have HT enabled.

The problem we have to address is that Intel has some MSRs shared
between threads and if HT is enabled we need to allocate some memory to
manage this shared resource.

The simple check outlined above X86_FEATURE_HT && smp_num_siblings > 1,
will get us most of the way and will I think be good enough (false
positives but no false negatives). But it did get us wondering about how
all this works.

So could you, irrespective of Linux implementation details tell us how
to detect if HT is available and enabled from a HW and BIOS perspective?

  reply	other threads:[~2011-01-04 11:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-27 15:36 [PATCH 4/7] perf: Check if HT is supported and enabled Lin Ming
     [not found] ` <AANLkTimp9VgD4qhOVmq-k1Ckzti9eeV8pf6Jvt5YB6nx@mail.gmail.com>
2010-12-28  8:51   ` Lin Ming
2011-01-03 10:58     ` Peter Zijlstra
2011-01-03 15:21       ` Andi Kleen
2011-01-03 19:53       ` H. Peter Anvin
2011-01-04 11:10         ` Peter Zijlstra [this message]
2011-01-04 13:38           ` Stephane Eranian
2011-01-04 13:44             ` Peter Zijlstra
2011-01-04 13:52               ` Stephane Eranian
2011-01-04 13:58                 ` Peter Zijlstra
2011-01-04 15:35                   ` Stephane Eranian
2011-01-04 19:00                   ` H. Peter Anvin
2011-01-05  5:45                 ` Lin Ming
2011-01-05 10:08                   ` Peter Zijlstra
2011-01-04 18:55           ` Valdis.Kletnieks

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=1294139419.2016.115.camel@laptop \
    --to=a.p.zijlstra@chello.nl \
    --cc=andi@firstfloor.org \
    --cc=eranian@google.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.m.lin@intel.com \
    --cc=mingo@elte.hu \
    /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