LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulose <Suzuki.Poulose@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	"linux@arm.linux.org.uk" <linux@arm.linux.org.uk>,
	"acme@kernel.org" <acme@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Punit Agrawal <Punit.Agrawal@arm.com>,
	Pawel Moll <Pawel.Moll@arm.com>
Subject: Re: [PATCH 1/3] arm/pmu: Reject groups spanning multiple hardware PMUs
Date: Tue, 10 Mar 2015 12:05:21 +0000	[thread overview]
Message-ID: <20150310120521.GD28168@leverpostej> (raw)
In-Reply-To: <20150310112723.GY2896@worktop.programming.kicks-ass.net>

On Tue, Mar 10, 2015 at 11:27:23AM +0000, Peter Zijlstra wrote:
> On Mon, Mar 09, 2015 at 12:46:30PM +0000, Suzuki K. Poulose wrote:
> > From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> > 
> > Don't allow grouping hardware events from different PMUs
> >  (eg. CCI + CPU).
> 
> Uhm, how does this work? If we have multiple hardware PMUs we'll stop
> scheduling events after the first failed event schedule. This can leave
> one of the PMUs severely under utilized.

The problem is here group validation at pmu::event_init() time, not
scheduling.

We don't allow grouping across disparate HW PMUs because we can't
provide group semantics anyway. Scheduling is not a problem in this case
(unlike the big.LITTLE case I have a patch for [1]).

We have a CPU PMU and an "uncore" CCI PMU. You can't create task-bound
events for the CCI, but you can create CPU-bound events for the CCI on
the nominal CPU the CCI is monitored from.

The context check you added in c3c87e770458aa00 "perf: Tighten (and fix)
the grouping condition" implicitly rejects groups that have CPU and CCI
events (each event::ctx will be the relevant pmu::pmu_cpu_context and
will differ), and this is sane -- you can't provide group semantics
across disparate HW PMUs.

Unfortunately that happens after we've done the
event->pmu->event_init(event) dance on each event, and in our event_init
function we try to verify the group is sane. In our verification we
ignore SW events, but assume that all !SW events are for the CPU PMU.
If you add a CPU event to a CCI group, that's not the case, and we use
container_of on an unsuitable object, derefence garbage, invoke the
eschaton and so on.

It would be nicer if we could prevent this in the core so we're not
reliant on every PMU driver doing the same verification. My initial
thought was that seemed like unnecessary duplication of the ctx checking
above, but if we're going to end up shoving it into several drivers
anyway perhaps it's the lesser evil.

Mark.

[1] https://lkml.org/lkml/2015/2/5/520

  parent reply	other threads:[~2015-03-10 12:05 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-09 12:46 [PATCH 0/3] [4.0] arm/arm64: Do not group hardware events from different PMUs Suzuki K. Poulose
2015-03-09 12:46 ` [PATCH 1/3] arm/pmu: Reject groups spanning multiple hardware PMUs Suzuki K. Poulose
2015-03-10 11:27   ` Peter Zijlstra
2015-03-10 12:00     ` Suzuki K. Poulose
2015-03-10 12:05     ` Mark Rutland [this message]
2015-03-10 12:53       ` Peter Zijlstra
2015-03-10 13:00         ` Peter Zijlstra
2015-03-10 13:57           ` Mark Rutland
2015-03-10 14:05           ` Suzuki K. Poulose
2015-03-10 15:09             ` Mark Rutland
2015-03-10 15:36         ` Mark Rutland
2015-03-10 15:44           ` Peter Zijlstra
2015-03-09 12:46 ` [PATCH 2/3] arm64/pmu: Reject groups spanning multiple HW PMUs Suzuki K. Poulose
2015-03-09 12:46 ` [PATCH 3/3] arm-cci: " Suzuki K. Poulose
  -- strict thread matches above, loose matches on Subject: below --
2015-03-09 12:43 [PATCH 0/3] [4.0] arm/arm64: Do not group hardware events from different PMUs a
2015-03-09 12:43 ` [PATCH 1/3] arm/pmu: Reject groups spanning multiple hardware PMUs a

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=20150310120521.GD28168@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=Pawel.Moll@arm.com \
    --cc=Punit.Agrawal@arm.com \
    --cc=Suzuki.Poulose@arm.com \
    --cc=Will.Deacon@arm.com \
    --cc=acme@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=peterz@infradead.org \
    --subject='Re: [PATCH 1/3] arm/pmu: Reject groups spanning multiple hardware PMUs' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).