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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120718114844.GH26120@redhat.com>
Date:	Wed, 18 Jul 2012 14:48:44 +0300
From:	Gleb Natapov <gleb@...hat.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	Alex Williamson <alex.williamson@...hat.com>, avi@...hat.com,
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
	jan.kiszka@...mens.com
Subject: Re: [PATCH v5 1/4] kvm: Extend irqfd to support level interrupts

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/