On Thu, Feb 18, 2010 at 12:18:25PM +0200, Avi Kivity wrote: > On 02/18/2010 12:05 PM, Jan Kiszka wrote: > >Avi Kivity wrote: > >>On 02/18/2010 11:45 AM, Avi Kivity wrote: > >>>On 02/18/2010 11:40 AM, Jan Kiszka wrote: > >>>>>Meanwhile, if anyone has any idea how to kill this lock, I'd love to > >>>>>see it. > >>>>> > >>>>What concurrency does it resolve in the end? On first glance, it only > >>>>synchronize the fiddling with pre-VCPU request bits, right? What forces > >>>>us to do this? Wouldn't it suffice to disable preemption (thus > >>>>migration) and the let concurrent requests race for setting the bits? I > >>>>mean if some request bit was already set on entry, we don't include the > >>>> related VCPU in smp_call_function_many anyway. > >>>It's more difficult. > >>> > >>>vcpu 0: sets request bit on vcpu 2 > >>> vcpu 1: test_and_set request bit on vcpu 2, returns already set > >>> vcpu 1: returns > >>>vcpu 0: sends IPI > >>>vcpu 0: returns > >>> > >>>so vcpu 1 returns before the IPI was performed. If the request was a > >>>tlb flush, for example, vcpu 1 may free a page that is still in vcpu > >>>2's tlb. > >>One way out would be to have a KVM_REQ_IN_PROGRESS, set it in > >>make_request, clear it in the IPI function. > >> > >>If a second make_request sees it already set, it can simply busy wait > >>until it is cleared, without sending the IPI. Of course the busy wait > >>means we can't enable preemption (or we may busy wait on an unscheduled > >>task), but at least the requests can proceed in parallel instead of > >>serializing. > > >...or include VCPUs with KVM_REQ_IN_PROGRESS set into the IPI set even > >if they already have the desired request bit set. > > But then we're making them take the IPI, which is pointless and > expensive. My approach piggy backs multiple requesters on one IPI. I have played with this in the past (collapsing that would avoid two simultaneous requestors from issuing two IPI's to a given vcpu, and unification with KVM_REQ_KICK to avoid the IPI if vcpu not in guest mode). Its not worthwhile though, this is not a contention point with TDP (maybe it becomes in the future with fine grained flushing, but not ATM). > >Then we should > >serialize in smp_call_function_many. > > Do you mean rely on s_c_f_m's internal synchronization? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
