Messages in this thread Patch in this message | | | Date | Thu, 27 Feb 2014 12:30:35 +0000 | From | Mark Rutland <> | Subject | Re: [PATCH 7/7] perf: kill perf_event_context::pmu |
| |
On Thu, Feb 27, 2014 at 11:51:43AM +0000, Peter Zijlstra wrote: > On Thu, Feb 27, 2014 at 11:48:05AM +0000, Mark Rutland wrote: > > On Tue, Feb 25, 2014 at 11:31:00AM +0000, Peter Zijlstra wrote: > > > On Tue, Feb 11, 2014 at 05:56:51PM +0000, Mark Rutland wrote: > > > > 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. > > > > > > No it does make sense; for example on hardware that doesn't have a PMI > > > you can create a software event + hardware event group and have the > > > software interrupt read the hardware counter and still get 'some' > > > sampling. > > > > Sure, I called out software events as an exception above. > > Oh sorry missed that. > > > Does it ever make sense to group two hardware events for disparate > > hardware PMUs? > > No, and I think we disallow that. We only explicitly allow software > events/groups to move to !software context.
As you say, other than the sw leader, hw follower case we currently check that event->ctx is equivalent. This works for homogeneous PMUs, as we only ever have one PMU in any task's perf_event_ctxp[perf_hw_context].
In the heterogeneous PMU case multiple CPU PMUs can share the perf_hw_context in a task, so the check for context equivalence is not sufficient.
I can hide the check in the architecture backend, or we could try to make the core code be more picky with something like the below.
---->8---- diff --git a/kernel/events/core.c b/kernel/events/core.c index 56003c6..a4b6f80 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -7135,6 +7135,8 @@ SYSCALL_DEFINE5(perf_event_open, if (group_leader->ctx->type != ctx->type) goto err_context; } else { + if (group_leader->pmu != pmu) + goto err_context; if (group_leader->ctx != ctx) goto err_context; } ---->8---- Cheers, Mark.
| |