On Tue, Nov 26, 2013 at 04:20:27PM +0100, Paolo Bonzini wrote: > Il 26/11/2013 16:03, Gleb Natapov ha scritto: > >>>> > >>I understood the proposal was also to eliminate the synchronize_rcu(), > >>>> > >>so while new interrupts would see the new routing table, interrupts > >>>> > >>already in flight could pick up the old one. > >>> > >Isn't that always the case with RCU? (See my answer above: "the vcpus > >>> > >already see the new routing table after the rcu_assign_pointer that is > >>> > >in kvm_irq_routing_update"). > >> > > >> > With synchronize_rcu(), you have the additional guarantee that any > >> > parallel accesses to the old routing table have completed. Since we > >> > also trigger the irq from rcu context, you know that after > >> > synchronize_rcu() you won't get any interrupts to the old > >> > destination (see kvm_set_irq_inatomic()). > > We do not have this guaranty for other vcpus that do not call > > synchronize_rcu(). They may still use outdated routing table while a vcpu > > or iothread that performed table update sits in synchronize_rcu(). > > Avi's point is that, after the VCPU resumes execution, you know that no > interrupt will be sent to the old destination because > kvm_set_msi_inatomic (and ultimately kvm_irq_delivery_to_apic_fast) is > also called within the RCU read-side critical section. > > Without synchronize_rcu you could have > > VCPU writes to routing table > e = entry from IRQ routing table > kvm_irq_routing_update(kvm, new); > VCPU resumes execution > kvm_set_msi_irq(e, &irq); > kvm_irq_delivery_to_apic_fast(); > > where the entry is stale but the VCPU has already resumed execution. > So how is it different from what we have now: disable_irq() VCPU writes to routing table e = entry from IRQ routing table kvm_set_msi_irq(e, &irq); kvm_irq_delivery_to_apic_fast(); kvm_irq_routing_update(kvm, new); synchronize_rcu() VCPU resumes execution enable_irq() receive stale irq -- Gleb. -- 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