public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Robert Richter <robert.richter@amd.com>
Cc: Stephane Eranian <eranian@google.com>,
	Ingo Molnar <mingo@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 00/10] perf, x86: Add northbridge counter support for AMD family 15h
Date: Wed, 20 Jun 2012 17:54:30 +0200	[thread overview]
Message-ID: <1340207670.21745.108.camel@twins> (raw)
In-Reply-To: <20120620122941.GH5046@erda.amd.com>

On Wed, 2012-06-20 at 14:29 +0200, Robert Richter wrote:
> On 20.06.12 12:16:13, Peter Zijlstra wrote:
> > Sure it can be done, just not pretty. Combine that with all the other
> > special casing like patches 3 and 10 and one really starts to wonder if
> > its all worth it.
> 
> I actually started writing the code by implementing a different pmu.
> It turned out to be the wrong direction. The pmus would be almost
> identical, just some different config values and a bit nb related
> special code. But you can't really reuse the functions on a 2nd
> running pmu, there are hard wired functions in the x86 pmu code and
> x86_pmu ops do not fit for such a split. It would mean a complete
> rework of x86 perf code. Really, I tried that already. And all this
> effort just to implement nb counters? If someone is willing to help
> here this would be ok, but I guess I would have to do all this on my
> own. And to be fair, this effort was also not make for fixed counters,
> pebs, bts, etc. Maybe the uncore implementation is different here, but
> today is the first day the uncore patches are in tip.

Yeah, the Intel uncore implements an entire new pmu. The code is a
little over the top because Intel went there and decided it was a good
thing to have numerous uncore pmus instead of 1, some in PCI space some
in MSR space.

Still their programming is similar to the core ones -- just like for
AMD.

Yeah, there's a little bit of 'duplicated' code, but that's unavoidable.

> I also do not see the advantage of a separate pmu. Just to have a
> different msr base to avoid the use of counter masks and some
> optimized pmu ops? Masks are wide spread used in the kernel and on x86
> the bsf instruction takes not more than an increment. And switches in
> the code paths to special nb code are not more expensive than other
> switches for other special code.

Well, as it stands this thing is almost certainly doing things wrong. An
uncore pmu wants to put all events for the same NB on the same cpu, not
on whatever cpu they are registered, otherwise event rotation doesn't
work right.

It also wants to migrate events to another cpu if the designated cpu
gets unplugged but there's still active cpus on the NB.

Furthermore, if the uncore does PMI, you want PMI steering, if it
doesn't do PMIs you want to poll the thing to avoid overflowing the
counter.

/me rummages on the interwebs to find the BKDG for Fam15h..

OK, it looks like it does do PMI and it broadcast interrupts to the
entire NB.. ok so that wants special magic too -- you might even want to
disallow sampling on the thing until someone has a good use-case for
that -- but you still need the PMI to deal with the counter overflow
stuff.




  reply	other threads:[~2012-06-20 15:54 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-19 18:10 [PATCH 00/10] perf, x86: Add northbridge counter support for AMD family 15h Robert Richter
2012-06-19 18:10 ` [PATCH 01/10] perf, amd: Rework northbridge event constraints handler Robert Richter
2012-06-19 18:10 ` [PATCH 02/10] perf, x86: Rework counter reservation code Robert Richter
2012-06-19 18:10 ` [PATCH 03/10] perf, x86: Use bitmasks for generic counters Robert Richter
2012-06-19 18:10 ` [PATCH 04/10] perf, x86: Rename Intel specific macros Robert Richter
2012-06-19 18:10 ` [PATCH 05/10] perf, x86: Move Intel specific code to intel_pmu_init() Robert Richter
2012-06-20  9:36   ` Peter Zijlstra
2012-06-20 14:22     ` Robert Richter
2012-06-19 18:10 ` [PATCH 06/10] perf, amd: Unify AMD's generic and family 15h pmus Robert Richter
2012-06-19 18:10 ` [PATCH 07/10] perf, amd: Generalize northbridge constraints code for family 15h Robert Richter
2012-06-19 18:10 ` [PATCH 08/10] perf, amd: Enable northbridge counters on " Robert Richter
2012-06-19 18:10 ` [PATCH 09/10] perf, x86: Improve debug output in check_hw_exists() Robert Richter
2012-06-19 18:10 ` [PATCH 10/10] perf, amd: Check northbridge event config value Robert Richter
2012-06-20  8:36 ` [PATCH 00/10] perf, x86: Add northbridge counter support for AMD family 15h Stephane Eranian
2012-06-20  8:54   ` Peter Zijlstra
     [not found] ` <CABPqkBS9hRxKLsecVK+AgRue6oqTtAg4=0Dpd5Z2VwAUja50fw@mail.gmail.com>
2012-06-20  9:29   ` Robert Richter
2012-06-20  9:38     ` Peter Zijlstra
2012-06-20 10:00       ` Robert Richter
2012-06-20 10:16         ` Peter Zijlstra
2012-06-20 12:29           ` Robert Richter
2012-06-20 15:54             ` Peter Zijlstra [this message]
2012-06-20 16:08               ` Peter Zijlstra
2012-06-20 16:21               ` Stephane Eranian
2012-06-20 10:46         ` Stephane Eranian
2012-06-20 12:41           ` Robert Richter
2012-06-20  9:41     ` Peter Zijlstra

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=1340207670.21745.108.camel@twins \
    --to=peterz@infradead.org \
    --cc=eranian@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=robert.richter@amd.com \
    /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