Re: [PATCH v5 1/4] kvm: Extend irqfd to support level interrupts

[Gleb Natapov]

On Wed, Jul 18, 2012 at 02:39:10PM +0300, Michael S. Tsirkin wrote:
> On Wed, Jul 18, 2012 at 02:22:19PM +0300, Michael S. Tsirkin wrote:
> > > > > > > > So as was discussed kvm_set_irq under spinlock is bad for scalability
> > > > > > > > with multiple VCPUs.  Why do we need a spinlock simply to protect
> > > > > > > > level_asserted?  Let's use an atomic test and set/test and clear and the
> > > > > > > > problem goes away.
> > > > > > > > 
> > > > > > > That sad reality is that for level interrupt we already scan all vcpus
> > > > > > > under spinlock.
> > > > > > 
> > > > > > Where?
> > > > > > 
> > > > > ioapic
> > > > 
> > > > $ grep kvm_for_each_vcpu virt/kvm/ioapic.c
> > > > $
> > > > 
> > > > ?
> > > > 
> > > 
> > > Come on Michael. You can do better than grep and actually look at what
> > > code does. The code that loops over all vcpus while delivering an irq is
> > > in kvm_irq_delivery_to_apic(). Now grep for that.
> > 
> > Hmm, I see, it's actually done for edge if injected from ioapic too,
> > right?
> > 
> > So set_irq does a linear scan, and for each matching CPU it calls
> > kvm_irq_delivery_to_apic which is another scan?
> > So it's actually N^2 worst case for a broadcast?
> 
> No it isn't, I misread the code.
> 
> 
> Anyway, maybe not trivially but this looks fixable to me: we could drop
> the ioapic lock before calling kvm_irq_delivery_to_apic.
> 
May be, may be not. Just saying "lets drop lock whenever we don't feel
like holding one" does not cut it. Back to original point though current
situation is that calling kvm_set_irq() under spinlock is not worse for
scalability than calling it not under one.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/