lkml.org 
[lkml]   [2010]   [Jan]   [25]   [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 Fri, 2010-01-22 at 21:27 +0100, Peter Zijlstra wrote:
> 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;
> > - }
> > -

I've split your -v6 delta in two, one part doing that fastpath
scheduling, and one part this hw_perf_enable optimization, for now I've
dropped the second part.

On top of that I did a patch that shares the above code with
x86_pmu_disable() so that we don't have that sequence twice.






\
 
 \ /
  Last update: 2010-01-25 16:03    [W:0.114 / U:1.852 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site