From: Dave Martin <Dave.Martin@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Will Deacon <Will.Deacon@arm.com>, Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH 7/7] perf: kill perf_event_context::pmu
Date: Wed, 12 Feb 2014 15:01:49 +0000 [thread overview]
Message-ID: <20140212150132.GA2231@e103592.cambridge.arm.com> (raw)
In-Reply-To: <20140211175651.GA15200@e106331-lin.cambridge.arm.com>
On Tue, Feb 11, 2014 at 05:56:51PM +0000, Mark Rutland wrote:
> On Mon, Feb 10, 2014 at 06:10:26PM +0000, Peter Zijlstra wrote:
> > On Mon, Feb 10, 2014 at 05:44:24PM +0000, Mark Rutland wrote:
> > > Currently portions of the perf subsystem assume that a
> > > perf_event_context is associated with a single pmu while in reality a
> > > single perf_event_context may be shared by a number of pmus, as commit
> > > 443772776c69 (perf: Disable all pmus on unthrottling and rescheduling)
> > > describes.
> > >
> > > This patch removes perf_event_context::pmu, replacing it with a direct
> > > pointer to the associated perf_cpu_context and a task_ctx_nr (as all
> > > pmus sharing a context have the same task_ctx_nr). This makes the
> > > relationship between pmus and perf_event_contexts clearer and allows us
> > > to save on some pointer chasing.
> > >
> > > This also fixes a potential misuse of ctx->pmu introduced in commit
> > > bad7192b842c (perf: Fix PERF_EVENT_IOC_PERIOD to force-reset the
> > > period), where ctx->pmu is disabled before modifying state on
> > > event->pmu. In this case the two pmus are not guaranteed to be the same.
> > >
> > > As perf_pmu_rotate_{start,stop} only really care about the context they
> > > are rotating, they are renamed to perf_event_ctx_{start,stop}.
> >
> > This very much relies on the previous patch where you make pmu_disable
> > iterate all the events.
> >
> > We could also change this to keep a pmu list for each context and
> > iterate that instead. Given there is indeed a fair limit on different
> > PMUs in the system that iteration should be much shorter.
>
> Another option would be to have a context per-pmu. Each context's pmu
> pointer would be valid, and (other than the case of software events) it
> doesn't make sense to place events from disparate PMUs into the same
> group anyway. Then you don't need a fixed sized pmu list in the context
> or some arcane list structs.
Getting event rotation to work in a sensible way when there are mixtures
of events for different PMUs in a single context. I've not come up with
a good enough solution for that to post yet.
Splitting out each PMU as a separate hardware context would make this
work a lot more naturally.
Cheers
---Dave
next prev parent reply other threads:[~2014-02-12 15:02 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-10 17:44 [PATCH 0/7] Perf core cleanups for shared perf_event_contexts Mark Rutland
2014-02-10 17:44 ` [PATCH 1/7] perf: fix prototype of find_pmu_context Mark Rutland
2014-02-27 13:33 ` [tip:perf/core] perf: Fix prototype of find_pmu_context() tip-bot for Mark Rutland
2014-02-10 17:44 ` [PATCH 2/7] perf: remove redundant pmu assignment Mark Rutland
2014-02-27 13:33 ` [tip:perf/core] perf: Remove redundant PMU assignment tip-bot for Mark Rutland
2014-02-10 17:44 ` [PATCH 3/7] perf: kill perf_event_context_type Mark Rutland
2014-02-25 11:38 ` Peter Zijlstra
2014-02-27 11:46 ` Mark Rutland
2014-02-10 17:44 ` [PATCH 4/7] perf: be less pessimistic when scheduling events Mark Rutland
2014-02-10 17:58 ` Peter Zijlstra
2014-02-11 17:48 ` Mark Rutland
2014-02-25 11:29 ` Peter Zijlstra
2014-02-27 12:07 ` Mark Rutland
2014-02-10 17:44 ` [PATCH 5/7] perf: kill pmu::hrtimer_interval_ms Mark Rutland
2014-02-10 17:44 ` [PATCH 6/7] perf: Centralise context pmu disabling Mark Rutland
2014-02-10 18:08 ` Peter Zijlstra
2014-02-10 17:44 ` [PATCH 7/7] perf: kill perf_event_context::pmu Mark Rutland
2014-02-10 18:10 ` Peter Zijlstra
2014-02-11 17:56 ` Mark Rutland
2014-02-12 15:01 ` Dave Martin [this message]
2014-02-25 11:31 ` Peter Zijlstra
2014-02-27 11:48 ` Mark Rutland
2014-02-27 11:51 ` Peter Zijlstra
2014-02-27 12:30 ` Mark Rutland
2014-02-19 13:43 ` [PATCH 0/7] Perf core cleanups for shared perf_event_contexts Mark Rutland
2014-02-25 11:39 ` 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=20140212150132.GA2231@e103592.cambridge.arm.com \
--to=dave.martin@arm.com \
--cc=Will.Deacon@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
/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
all inboxes | Powered by JetHome®