lkml.org 
[lkml]   [2010]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] perf_events: improve x86 event scheduling (v6 incremental)
From
Date
On Thu, 2010-01-21 at 17:39 +0200, Stephane Eranian wrote:
> @@ -1395,40 +1430,28 @@ void hw_perf_enable(void)
> * apply assignment obtained either from
> * hw_perf_group_sched_in() or x86_pmu_enable()
> *
> - * step1: save events moving to new counters
> - * step2: reprogram moved events into new counters
> + * We either re-enable or re-program and re-enable.
> + * All events are disabled by the time we come here.
> + * That means their state has been saved already.
> */

I'm not seeing how it is true.

Suppose a core2 with counter0 active counting a non-restricted event,
say cpu_cycles. Then we do:

perf_disable()
hw_perf_disable()
intel_pmu_disable_all
wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, 0);

->enable(MEM_LOAD_RETIRED) /* constrained to counter0 */
x86_pmu_enable()
collect_events()
x86_schedule_events()
n_added = 1
/* also slightly confused about this */
if (hwc->idx != -1)
x86_perf_event_set_period()
perf_enable()
hw_perf_enable()

/* and here we'll assign the new event to counter0
* except we never disabled it... */
intel_pmu_enable_all()
wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, intel_ctrl)

Or am I missing something?

> for(i=0; i < cpuc->n_events; i++) {
>
> event = cpuc->event_list[i];
> hwc = &event->hw;
>
> - if (hwc->idx == -1 || hwc->idx == cpuc->assign[i])
> - continue;
> -
> - x86_pmu.disable(hwc, hwc->idx);
> -
> - clear_bit(hwc->idx, cpuc->active_mask);
> - barrier();
> - cpuc->events[hwc->idx] = NULL;
> -
> - x86_perf_event_update(event, hwc, hwc->idx);
> -
> - hwc->idx = -1;
> - }
> -
> - for(i=0; i < cpuc->n_events; i++) {
> -
> - event = cpuc->event_list[i];
> - hwc = &event->hw;
> -
> - if (hwc->idx == -1) {
> - x86_assign_hw_event(event, hwc, cpuc->assign[i]);
> + /*
> + * we can avoid reprogramming counter if:
> + * - assigned same counter as last time
> + * - running on same CPU as last time
> + * - no other event has used the counter since
> + */
> + if (!match_prev_assignment(hwc, cpuc, i)) {
> + x86_assign_hw_event(event, cpuc, cpuc->assign[i]);
> x86_perf_event_set_period(event, hwc, hwc->idx);
> }
> /*
> * need to mark as active because x86_pmu_disable()
> - * clear active_mask and eventsp[] yet it preserves
> + * clear active_mask and events[] yet it preserves
> * idx
> */
> set_bit(hwc->idx, cpuc->active_mask);



\
 
 \ /
  Last update: 2010-01-22 21:31    [from the cache]
©2003-2011 Jasper Spaans