[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Xen HPET improvement proposal



>>> On 28.10.13 at 12:20, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>>>> On 25.10.13 at 15:11, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 25/10/13 13:47, Jan Beulich wrote:
>>>
>>>> Independently of the HPET issues themselves, I have identified a race
>>>> condition in the mwait-idle routines where a cpu which is preparing to
>>>> sleep can arrange for another cpu to wake it up, and have that other cpu
>>>> wake it up before it has enabled its mwait trigger, meaning that it will
>>>> idle for an arbitrary length of time in mwait.  Realistically, the cpu
>>>> will be woken up by the time calibration rendezvous once a second, and
>>>> possibly by the watchdog NMI every half second.
>>> Which is an awfully long period of time... Looking forward to see
>>> further details on this.
>> 
>> The fix is fairly simple.  The mwait code must set up the trigger on its
>> mwait region before arranging to be woken up.  That way, if the other
>> cpu does wake up (early perhaps), it will activate the trigger, and we
>> will bounce straight back out of mwait rather than sleeping indefinitely.
> 
> Actually I think rather than setting the trigger earlier (i.e. moving
> the invocation of __monitor() out of mwait_idle_with_hints()) we
> should rather check for an already occurred wakeup right after
> having invoked __monitor(). Since the boolean-ness of the variable
> pointed to by mwait_wakeup() is currently unused, we could easily
> use that one. Patch in the works...

But then again - are you sure there is a race? _After_ having called
__monitor(), mwait_idle_with_hints() checks another time that the
expiry time indeed still is in the future.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.