On Wed, 2010-01-13 at 10:54 +0100, Stephane Eranian wrote:
> > One concern I do have is the loss of error checking on
> > event_sched_in()'s event->pmu->enable(), that could be another
> > 'hardware' PMU like breakpoints, in which case it could fail.
> >
> Well, x86_pmu_enable() does return an error code, so it is up
> to the upper layer to handle the error gracefully and in particular
> in perf_ctx_adjust_freq().
> +static void event_sched_in(struct perf_event *event, int cpu)
> +{
> + event->state = PERF_EVENT_STATE_ACTIVE;
> + event->oncpu = cpu;
> + event->tstamp_running += event->ctx->time - event->tstamp_stopped;
> + if (is_software_event(event))
> + event->pmu->enable(event);
> +}
> +
> +/*
> + * Called to enable a whole group of events.
> + * Returns 1 if the group was enabled, or -EAGAIN if it could not be.
> + * Assumes the caller has disabled interrupts and has
> + * frozen the PMU with hw_perf_save_disable.
> + *
> + * called with PMU disabled. If successful and return value 1,
> + * then guaranteed to call perf_enable() and hw_perf_enable()
> + */
> +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 n, n0, ret;
> +
> + n0 = cpuc->n_events;
> +
> + n = collect_events(cpuc, leader, true);
> + if (n < 0)
> + return n;
> +
> + ret = x86_schedule_events(cpuc, n, assign);
> + if (ret)
> + return ret;
> +
> + /*
> + * copy new assignment, now we know it is possible
> + * will be used by hw_perf_enable()
> + */
> + memcpy(cpuc->assign, assign, n*sizeof(int));
> +
> + cpuc->n_events = n;
> + cpuc->n_added = n - n0;
> +
> + n = 1;
> + event_sched_in(leader, cpu);
> + list_for_each_entry(sub, &leader->sibling_list, group_entry) {
> + if (sub->state != PERF_EVENT_STATE_OFF) {
> + event_sched_in(sub, cpu);
> + ++n;
> + }
> + }
> + ctx->nr_active += n;
> +
> + /*
> + * 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;
> +}
That most certainly looses error codes for all !is_x86_event() events in
the group.
So you either need to deal with that event_sched_in() failing, or
guarantee that it always succeeds (forcing software events only for
example).
------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
