[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1284483392.2275.540.camel@laptop>
Date: Tue, 14 Sep 2010 18:56:32 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Paul Mackerras <paulus@...ba.org>
Cc: stephane eranian <eranian@...glemail.com>,
Robert Richter <robert.richter@....com>,
Will Deacon <will.deacon@....com>,
Paul Mundt <lethal@...ux-sh.org>,
Frederic Weisbecker <fweisbec@...il.com>,
Cyrill Gorcunov <gorcunov@...il.com>,
Lin Ming <ming.m.lin@...el.com>,
Yanmin <yanmin_zhang@...ux.intel.com>,
Deng-Cheng Zhu <dengcheng.zhu@...il.com>,
David Miller <davem@...emloft.net>,
Ingo Molnar <mingo@...e.hu>, Michael Cree <mcree@...on.net.nz>,
linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH 00/19] perf pmu interface changes -v4
On Fri, 2010-09-10 at 14:39 +1000, Paul Mackerras wrote:
> On Tue, Sep 07, 2010 at 06:46:33PM +0200, Peter Zijlstra wrote:
>
> > These patches prepare the perf code for multiple PMUs.
> >
> > About the first half of these patches deals with removing all the weak
> > functions, the second half provides per pmu contexts.
> >
> > New to the first half is Alpha, which got merged into mainline recently.
> >
> > The second half is completely new and lightly tested, one known breakage is
> > that it currently doesn't allow to mix software and hardware events into a
> > single group (should be fixed soon).
>
> I see Ingo committed this series into the tip perf/core branch. Did
> this problem get fixed before that, or does it still need to be fixed?
I was thinking something like the below, except I obviously need to sort
that XXX thing.
---
kernel/perf_event.c | 48 ++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 40 insertions(+), 8 deletions(-)
Index: linux-2.6/kernel/perf_event.c
===================================================================
--- linux-2.6.orig/kernel/perf_event.c
+++ linux-2.6/kernel/perf_event.c
@@ -494,7 +494,7 @@ static void __perf_event_remove_from_con
raw_spin_lock(&ctx->lock);
- event_sched_out(event, cpuctx, ctx);
+ group_sched_out(event, cpuctx, ctx);
list_del_event(event, ctx);
@@ -5522,9 +5522,10 @@ SYSCALL_DEFINE5(perf_event_open,
struct perf_event_context *ctx;
struct file *event_file = NULL;
struct file *group_file = NULL;
- struct task_struct *task;
+ struct task_struct *task = NULL;
struct pmu *pmu;
int event_fd;
+ int move_group_leader = 0;
int fput_needed = 0;
int err;
@@ -5574,15 +5575,34 @@ SYSCALL_DEFINE5(perf_event_open,
* any hardware group.
*/
pmu = event->pmu;
- if ((pmu->task_ctx_nr == perf_sw_context) && group_leader)
- pmu = group_leader->pmu;
+
+ if (group_leader &&
+ (is_software_event(event) != is_software_event(group_leader))) {
+ if (is_software_event(event)) {
+ /*
+ * If event and group_leader are not both a software
+ * event, and event it, then group leader is not.
+ *
+ * Allow the addition of software events to !software
+ * groups, this is safe because software events never
+ * fail to schedule.
+ */
+ pmu = group_leader->pmu;
+ } else if (is_software_event(group_leader) &&
+ (group_leader->group_flags & PERF_GROUP_SOFTWARE)) {
+ /*
+ * In case the group is a pure software group, and we
+ * try to add a hardware event, move the whole group to
+ * the hardware context.
+ */
+ move_group_leader = 1;
+ }
+ }
/*
* Get the target context (task or percpu):
*/
- if (pid == -1 && cpu != -1)
- task = NULL;
- else
+ if (!(pid == -1 && cpu != -1))
task = find_lively_task_by_vpid(pid);
ctx = find_get_context(pmu, task, cpu);
@@ -5606,8 +5626,10 @@ SYSCALL_DEFINE5(perf_event_open,
/*
* Do not allow to attach to a group in a different
* task or CPU context:
+ *
+ * XXX fix the task vs cpu context thing for move_group_leader
*/
- if (group_leader->ctx != ctx)
+ if (!move_group_leader && group_leader->ctx != ctx)
goto err_context;
/*
* Only a group leader can be exclusive or pinned
@@ -5628,9 +5650,19 @@ SYSCALL_DEFINE5(perf_event_open,
goto err_context;
}
+ if (move_group_leader) {
+ struct perf_event_context *gctx = group_leader->ctx;
+
+ mutex_lock(&gctx->mutex);
+ perf_event_remove_from_context(group_leader);
+ mutex_unlock(&gctx->mutex);
+ }
+
event->filp = event_file;
WARN_ON_ONCE(ctx->parent_ctx);
mutex_lock(&ctx->mutex);
+ if (move_group_leader)
+ perf_install_in_context(ctx, group_leader, cpu);
perf_install_in_context(ctx, event, cpu);
++ctx->generation;
mutex_unlock(&ctx->mutex);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists