[Xen-devel] RE: [PATCH] CPUIDLE: shorten hpet spin_lock holding time

["Wei, Gang" <gang.wei@xxxxxxxxx>]

On Wednesday, 2010-4-21 5:26 PM, Keir Fraser wrote:
> On 21/04/2010 10:06, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote:
> 
>>> It fixes the unsafe accesses to timer_deadline_{start,end} but I
>>> still think this optimisation is misguided and also unsafe. There is
>>> nothing to stop new CPUs being added to ch->cpumask after you start
>>> scanning ch->cpumask. For example, some new CPU which has a
>>> timer_deadline_end greater than ch->next_event, so it does not
>>> reprogram the HPET. But handle_hpet_broadcast is already mid-scan
>>> and misses this new CPU, so it does not reprogram the HPET either.
>>> Hence no timer fires for the new CPU and it misses its deadline.
>> 
>> This will not happen. ch->next_event has already been set as
>> STIME_MAX before start scanning ch->cpumask, so the new CPU with
>> smallest timer_deadline_end will reprogram the HPET successfully.
> 
> Okay, then CPU A executes hpet_broadcast_enter() and programs the HPET
> channel for its timeout X. Meanwhile concurrently executing
> handle_hpet_broadcast misses CPU A but finds some other CPU B with
> timeout Y much later than X, and erroneously programs the HPET
> channel with Y, causing CPU A to miss its deadline by an arbitrary
> amount. 

It is also not possible. handle_hpet_broadcast reprogram HPET just while 
next_event < ch->next_event. Here next_evet is Y, and ch->next_event is X. :-)

> I dare say I can carry on finding races. :-)

Please go on... The two racing conditions you mentioned were just considered 
before I resent the patch. :-D

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel