On Mon, Jan 18, 2010 at 10:58:01AM +0200, Stephane Eranian wrote:
> +int hw_perf_group_sched_in(struct perf_event *leader,
> + struct perf_cpu_context *cpuctx,
> + struct perf_event_context *ctx, int cpu)
> +{
> + struct cpu_hw_events *cpuc = &per_cpu(cpu_hw_events, cpu);
> + struct perf_event *sub;
> + int assign[X86_PMC_IDX_MAX];
> + int n0, n1, ret;
> +
> + /* n0 = total number of events */
> + n0 = collect_events(cpuc, leader, true);
> + if (n0 < 0)
> + return n0;
> +
> + ret = x86_schedule_events(cpuc, n0, assign);
> + if (ret)
> + return ret;
> +
> + ret = x86_event_sched_in(leader, cpuctx, cpu);
> + if (ret)
> + return ret;
> +
> + n1 = 1;
> + list_for_each_entry(sub, &leader->sibling_list, group_entry) {
> + if (sub->state != PERF_EVENT_STATE_OFF) {
> + ret = x86_event_sched_in(sub, cpuctx, cpu);
> + if (ret)
> + goto undo;
> + ++n1;
> + }
> + }
> + /*
> + * copy new assignment, now we know it is possible
> + * will be used by hw_perf_enable()
> + */
> + memcpy(cpuc->assign, assign, n0*sizeof(int));
> +
> + cpuc->n_events = n0;
> + cpuc->n_added = n1;
> + ctx->nr_active += n1;
> +
> + /*
> + * 1 means successful and events are active
> + * This is not quite true because we defer
> + * actual activation until hw_perf_enable() but
> + * this way we* ensure caller won't try to enable
> + * individual events
> + */
> + return 1;
> +undo:
> + x86_event_sched_out(leader, cpuctx, cpu);
> + n0 = 1;
> + list_for_each_entry(sub, &leader->sibling_list, group_entry) {
> + if (sub->state == PERF_EVENT_STATE_ACTIVE) {
> + x86_event_sched_out(sub, cpuctx, cpu);
> + if (++n0 == n1)
> + break;
> + }
> + }
> + return ret;
Looking at all these numerous places where this whole constraint
and ad hoc scheduling machine tries to catch up with what the
core can already do (handle non-x86 events, revert in failure,
first handle leader, then handle the rest, etc...), I wonder
if this hw_group_sched_in() based design is a good idea.
Shouldn't we actually use the core based pmu->enable(),disable()
model called from kernel/perf_event.c:event_sched_in(),
like every other events, where we can fill up the queue of hardware
events to be scheduled, and then call a hw_check_constraints()
when we finish a group scheduling?
I guess this would simplify all this code, avoid it to run through
the list of events, handle software events, revert partial enabled
by itself etc...
Hm?
------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
