[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] RE: [PATCH] CPUIDLE: shorten hpet spin_lock holding time
On Tuesday, 2010-4-20 10:22 PM, Keir Fraser wrote: > On 20/04/2010 15:04, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote: > >> On Tuesday, 2010-4-20 8:49 PM, Keir Fraser wrote: >>> Is this a measurable win? The newer locking looks like it could be >>> dodgy on 32-bit Xen: the 64-bit reads of timer_deadline_{start,end} >>> will be non-atomic and unsynchronised so you can read garbage. Even >>> on 64-bit Xen you can read stale values. I'll be surprised if you >>> got a performance win from chopping up critical regions in >>> individual functions like that anyway. >> >> First of all, this is a measurable power win for cpu overcommitment >> idle case (vcpu:pcpu > 2:1, pcpu >= 32, guests are non-tickless >> kernels). > > So lots of short sleep periods, and possibly only a very few HPET > channels to share? Yes. > How prevalent is always-running APIC timer now, > and is that going to be supported in future processors? Always-running APIC timer just started from Westmere. And I personally guess it should be supported in future processors. There are a lot of existing system still need hpet broadcast wakeup. >> As to the non-atomic access to timer_deadline_{start,end}, it should >> already be there before this patch. It is not protected by the hpet >> lock. Shall we add rw_lock for each timer_deadline_{start,end}? This >> can be done separately. > > The bug isn't previously there, since the fields will not be read > unless the cpu is in ch->cpumask, which (was) protected by ch->lock. > That was sufficient since a CPU would not modify > timer_deadline_{start,end} between hpet_broadcast_enter and > hpet_broadcast_exit. After your patch, handle_hpet_broadcast is no > longer fully synchronised against those functions. Ok, your are right. So we may just need to make sure the cpu is in ch->cpumask while reading the timer_deadline_{start,end}. I can make some changes to my patch. Jimmy _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |