public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	linux-kernel@vger.kernel.org, tglx@linutronix.de,
	linux-tip-commits@vger.kernel.org
Subject: Re: [tip:perf/core] perf/x86: Clean up cap_user_time* setting
Date: Mon, 07 Oct 2013 11:30:34 -0700	[thread overview]
Message-ID: <5252FDCA.2080300@zytor.com> (raw)
In-Reply-To: <20131007174037.GI3081@twins.programming.kicks-ass.net>

On 10/07/2013 10:40 AM, Peter Zijlstra wrote:
> On Mon, Oct 07, 2013 at 10:22:06AM -0700, H. Peter Anvin wrote:
>> CONFIG_X86_TSC is a baseline control option; we shouldn't key
>> functionality off of it.  It's fine to say notsc -> no tracing, but
>> making it a compile-time key makes me a bit uphappy.  We cut off 386,
>> but cutting of 486 at this point makes me nervous.
> 
> The thing that annoys me about notsc is that it disables usage even if
> its present.

That is *exactly* what it is supposed to do.  It seems to do so poorly
at this point.  It should behave exactly as if the TSC isn't there (and
thus any feature which depends on the TSC is not available, either.)

> I've no problem with 486 which simply doesn't have TSC and thus
> cpu_has_tsc will be false and other stuff will happen -- and I don't
> think my proposition would actually change anything there.

Except it will be a lot harder to test it.

> What is completely insane is people using notsc on say a haswell chip
> and expecting something sane to happen.
>
> So I'm not proposing we remove !cap_has_tsc support; all I'm proposing
> is we remove the notsc knob that avoids using the TSC on perfectly good
> hardware.

Uh, no, that's broken.

There seem to be a couple of issues here:

1. We have keyed functionality off CONFIG_X86_TSC, which is a baseline
control option -- this is basically a no-no.

2. With CONFIG_X86_TSC turned on, notsc is meaningless: it means running
in a configuration that the kernel compile doesn't support

3. Functionality that depends on the TSC should be disabled at runtime
if !CONFIG_X86_TSC.

4. Does CONFIG_X86_TSC even make sense?  It isn't listed in
required-features.h and is only present in a handful of places in the
kernel, and the trace-clock bits seem just plain wrong.  It isn't
valuable to unload TSC support (which isn't what CONFIG_X86_TSC does as
it is currently configured) even on embedded (non-TSC hardware is rare
today)... so why do we even bother?

	-hpa


      reply	other threads:[~2013-10-07 18:31 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-04 17:31 [tip:perf/core] perf/x86: Clean up cap_user_time* setting tip-bot for Peter Zijlstra
2013-10-04 18:31 ` Adrian Hunter
2013-10-04 18:55   ` Peter Zijlstra
2013-10-06  9:10     ` Ingo Molnar
2013-10-07  9:33       ` Peter Zijlstra
2013-10-07 15:59         ` Peter Zijlstra
2013-10-07 17:22       ` H. Peter Anvin
2013-10-07 17:40         ` Peter Zijlstra
2013-10-07 18:30           ` H. Peter Anvin [this message]

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=5252FDCA.2080300@zytor.com \
    --to=hpa@zytor.com \
    --cc=adrian.hunter@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --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