Il 31/05/2013 11:18, Gleb Natapov ha scritto: > On Fri, May 31, 2013 at 10:48:32AM +0200, Paolo Bonzini wrote: >> Il 31/05/2013 06:36, Gleb Natapov ha scritto: >>> In my commit message there is two INITs in a row: >>> vpu0: vcpu1: >>> set INIT >>> test_and_clear_bit(KVM_APIC_INIT) >>> process INIT >>> set INIT >>> set SIPI >>> test_and_clear_bit(KVM_APIC_SIPI) >>> process SIPI >>> >>> Two INITs before SIPI are essential to trigger the bug >> >> I see now. Let's draw pending_events as well: >> >> event sent event processed pending_events >> INIT INIT >> INIT 0 >> INIT INIT >> SIPI INIT|SIPI >> SIPI INIT >> INIT 0 >> >> Events are reordered, there is indeed a bug if the second INIT comes at >> just the right time. With your patch: >> >> event sent event processed pending_events >> INIT INIT >> INIT 0 >> INIT INIT >> SIPI INIT|SIPI >> SIPI, failed cmpxchg INIT|SIPI >> INIT SIPI >> SIPI SIPI > > This is incorrect. cmpxchg will fail only if another INIT cames after SIPI. > Why would it fail? You're right. Can you show what is the case in my patch where you have coalescing? I still prefer it because it is a smaller change, it keeps the "clear a bit before processing" idea that you find almost everywhere. Changing it to "clear a bit after processing" is a bigger and more surprising change, though both are indeed tricky. Paolo -- 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
