Messages in this thread | | | Date | Tue, 9 Jun 2015 18:59:36 +0800 | From | Huang Rui <> | Subject | Re: [PATCH v2 3/4] x86, mwaitt: introduce mwaix delay with a configurable timer |
| |
On Tue, Jun 09, 2015 at 05:29:52PM +0800, Peter Zijlstra wrote: > On Tue, Jun 09, 2015 at 11:13:40AM +0800, Huang Rui wrote: > > +static void delay_mwaitx(unsigned long __loops) > > +{ > > + u32 end, now, delay, addr; > > + > > + delay = __loops; > > + rdtsc_barrier(); > > + rdtscl(end); > > + end += delay; > > + > > + while (1) { > > + __monitorx(&addr, 0, 0); > > + mwaitx(delay, true); > > + > > + rdtsc_barrier(); > > + rdtscl(now); > > + if (end <= now) > > + break; > > + delay = end - now; > > + } > > How about you think instead and do something like: > > rdtsc(start); > rdtsc_barrier(); > > for (;;) { > delay = min(MWAIT_MAX_LOOPS, loops); > > __monitorx(&addr, 0, 0); > mwaitx(delay, true); > > rdtsc_barrier(); > rdtsc(end); > rdtsc_barrier(); > > loops -= end - start; > if (loops <= 0) > break; > > start = end; > } >
It looks better, thanks. :)
> > +} > > + > > +/* > > * Since we calibrate only once at boot, this > > * function should be set once at boot and not changed > > */ > > @@ -118,7 +145,12 @@ int read_current_timer(unsigned long *timer_val) > > > > void __delay(unsigned long loops) > > { > > - delay_fn(loops); > > + if (loops > MWAITX_MAX_LOOPS || > > + !static_cpu_has_safe(X86_FEATURE_MWAITT) || > > + boot_option_delay != DELAY_MWAITX) > > + delay_fn(loops); > > + else > > + delay_mwaitx(loops); > > } > > Then you can do away with that fallback entirely.
OK, I will submit V3 to remove kernel parameter and with the change of delay method.
Thanks, Rui
| |