Re: [PATCH 7/7] perf: kill perf_event_context::pmu

[Mark Rutland]

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.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/