Messages in this thread | | | From | Andy Lutomirski <> | Date | Thu, 21 May 2015 10:12:32 -0700 | Subject | Re: [RFC PATCH 2/4] x86, mwaitt: introduce mwaitx idle with a configurable timer |
| |
On Thu, May 21, 2015 at 10:08 AM, Borislav Petkov <bp@suse.de> wrote: > On Thu, May 21, 2015 at 09:45:10AM -0700, Andy Lutomirski wrote: >> I must be missing something. In this sequence, you're sleeping with >> IF=0 and ECX[0] = 0, so an IRQ won't get handled. Don't we want >> ECX[0] = 1? > > Hmm, so actually we don't want to sleep with interrupts disabled. If > ECX[0]=1b, then an interrupt will wake MWAIT. So then you have to do the > loop thing as tglx suggested. > >> > The NMI argument is a problem though - if and NMI gets you out of >> > MWAITX, a simple perf tool workload would kill all MWAITX executions. >> > Which is bad. :-\ >> >> I'm not sure it's a show-stopper. NMI handlers are meant to be fast. >> If an NMI comes in between rdtsc and mwaitx, then we oversleep, but by >> at most the time it takes to handle an NMI, and nothing would have >> stopped us from oversleeping that long if an NMI came in right after >> mwaitx returned. > > Actually, I'm thinking about an NMI happening after we've issued MWAIT. > NMIs wake it up. So you have the same problem as above: > > NMIs will wake MWAIT so you'd need to check how long you've slept and > sleep for the remaining time. I.e., something like that thing from a > couple of mails ago: > > delay = usec_to_tsc(delay_usec); > > if (delay > ((1 << 32) - 1)) { > mdelay(delay_usec); > return; > } > > end = rdtsc() + delay; > while (1) { > > monitorx( ...); /* Do we need that here? */ > mwaitx(delay); > > /* possible wakeups */ > > now = rdtsc(); > if (end <= now) > break; > delay = end - now; > } > > > Yes, no?
Yes, but there should already be an adequate outer loop around the whole thing. After all, even regular mwait can have spurious wakeups due to monitor monitoring the entire cache line.
--Andy
| |