LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Suzuki Poulose <Suzuki.Poulose@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	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 15:09:23 +0000	[thread overview]
Message-ID: <20150310150923.GA31849@leverpostej> (raw)
In-Reply-To: <54FEFA23.7050307@arm.com>

> I think we could still solve this problem by deferring the 'context'
> validation to the core. The PMUs could validate the group, within its
> context. i.e, if it can accommodate its events as a group, during 
> event_init.  The problem we face now, is encountering an event from a 
> different PMU, which we could leave it to the core as we do already.

Good point: we're not reliant on other drivers because the core will
still check the context. We only hope that those other drivers don't
make similar mistakes and corrupt things.

[...]

>   static int
> -validate_event(struct pmu_hw_events *hw_events,
> -	       struct perf_event *event)
> +validate_event(struct pmu *pmu, struct pmu_hw_events *hw_events,
> +			       struct perf_event *event)
>   {
> -	struct arm_pmu *armpmu = to_arm_pmu(event->pmu);
> +	struct arm_pmu *armpmu;
> 
>   	if (is_software_event(event))
>   		return 1;
> 
> +	/*
> +	 * We are only worried if we can accommodate the events
> +	 * from this pmu in this group.
> +	 */
> +	if (event->pmu != pmu)
> +		return 1;

It's better to explicitly reject this case. We know it's non-sensical
and there's no point wasting any time on it. That will also make
big.LITTLE support a bit nicer, whenever I get that under control -- big
and LITTLE events could live in the same task context (so the core won't
reject grouping them) but mustn't be in the same group (so we have to
reject grouping in the backend).

I'd still prefer the group validation being triggered explicitly by the
core code, so that it's logically separate from initialising the event
in isolation, but that's looking like a much bigger job, and I don't
trust myself to correctly update every PMU driver for v4.0.

For the moment let's clean up the commit message for the original patch.
I'll add splitting group validation to my TODO list; there seems to be a
slot free around 2035...

Thanks,
Mark.

  reply	other threads:[~2015-03-10 15:09 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
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 [this message]
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=20150310150923.GA31849@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).