Messages in this thread | | | Subject | Re: [PATCH 4/5] perf: Introduce task, softirq and hardirq contexts exclusion | From | Peter Zijlstra <> | Date | Thu, 10 Jun 2010 13:01:27 +0200 |
| |
On Thu, 2010-06-10 at 05:49 +0200, Frederic Weisbecker wrote: > @@ -642,17 +643,31 @@ event_sched_in(struct perf_event *event, > if (event->state <= PERF_EVENT_STATE_OFF) > return 0; > > - event->state = PERF_EVENT_STATE_ACTIVE; > + if (event->attr.exclude_task) > + event->state = PERF_EVENT_STATE_PAUSED; > + else > + event->state = PERF_EVENT_STATE_ACTIVE; > + > event->oncpu = smp_processor_id(); > +
Aah, so that is why you added the PAUSE state knowledge to the arch code, you want to be able to call ->enable() on a PAUSEd event.
That means you need to audit/touch all implementations anyway, isn't there a better interface we can use, like maybe extend ->enable() with a flags argument?
> /* > * The new state must be visible before we turn it on in the hardware: > */ > smp_wmb(); > > - if (event->pmu->enable(event)) { > - event->state = PERF_EVENT_STATE_INACTIVE; > - event->oncpu = -1; > - return -EAGAIN; > + /* > + * If we exclude the tasks, we only need to schedule hardware > + * events that need to settle themselves, even in a pause mode. > + * Software events can simply be scheduled anytime. > + * If we want more granularity in all that, we can still provide > + * later a pmu->reserve callback. > + */ > + if (!event->attr.exclude_task || !is_software_event(event)) { > + if (event->pmu->enable(event)) { > + event->state = PERF_EVENT_STATE_INACTIVE; > + event->oncpu = -1; > + return -EAGAIN; > + } > } > > event->tstamp_running += ctx->time - event->tstamp_stopped;
Remove is_software_event(), not add more.
| |