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.





------------------------------------------------------------------------------
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

Reply via email to