[PATCH] kvm: only allow one gsi per fd

[PATCH] kvm: only allow one gsi per fd

[Michael S. Tsirkin]

Looks like repeatedly binding same fd to multiple gsi's with irqfd can
use up a ton of kernel memory for irqfd structures.

A simple fix is to allow each fd to only trigger one gsi: triggering a
srorm of interrupts in guest is likely useless anyway, and we can do it
by binding a single gsi to many interrupts if we really want to.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

This patch is IMO a good candidate for 2.6.33 and 2.6.32.x.

 virt/kvm/eventfd.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 30f70fd..62e4cd9 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -166,7 +166,7 @@ irqfd_ptable_queue_proc(struct file *file, wait_queue_head_t *wqh,
 static int
 kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi)
 {
-	struct _irqfd *irqfd;
+	struct _irqfd *irqfd, *tmp;
 	struct file *file = NULL;
 	struct eventfd_ctx *eventfd = NULL;
 	int ret;
@@ -203,9 +203,20 @@ kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi)
 	init_waitqueue_func_entry(&irqfd->wait, irqfd_wakeup);
 	init_poll_funcptr(&irqfd->pt, irqfd_ptable_queue_proc);
 
+	spin_lock_irq(&kvm->irqfds.lock);
+
+	ret = 0;
+	list_for_each_entry(tmp, &kvm->irqfds.items, list) {
+		if (irqfd->eventfd != tmp->eventfd)
+			continue;
+		/* This fd is used for another irq already. */
+		ret = -EBUSY;
+		spin_unlock_irq(&kvm->irqfds.lock);
+		goto fail;
+	}
+
 	events = file->f_op->poll(file, &irqfd->pt);
 
-	spin_lock_irq(&kvm->irqfds.lock);
 	list_add_tail(&irqfd->list, &kvm->irqfds.items);
 	spin_unlock_irq(&kvm->irqfds.lock);
 
-- 
1.6.6.144.g5c3af

Re: [PATCH] kvm: only allow one gsi per fd

[Gregory Haskins]

>>> On 1/13/2010 at 11:58 AM, in message <20100113165809.GA13334@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com> wrote: 
> Looks like repeatedly binding same fd to multiple gsi's with irqfd can
> use up a ton of kernel memory for irqfd structures.
> 
> A simple fix is to allow each fd to only trigger one gsi: triggering a
> srorm of interrupts in guest is likely useless anyway, and we can do it
> by binding a single gsi to many interrupts if we really want to.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Seems reasonable to me.

Acked-by: Gregory Haskins <ghaskins@novell.com>

> ---
> 
> This patch is IMO a good candidate for 2.6.33 and 2.6.32.x.
> 
>  virt/kvm/eventfd.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 30f70fd..62e4cd9 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -166,7 +166,7 @@ irqfd_ptable_queue_proc(struct file *file, 
> wait_queue_head_t *wqh,
>  static int
>  kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi)
>  {
> -	struct _irqfd *irqfd;
> +	struct _irqfd *irqfd, *tmp;
>  	struct file *file = NULL;
>  	struct eventfd_ctx *eventfd = NULL;
>  	int ret;
> @@ -203,9 +203,20 @@ kvm_irqfd_assign(struct kvm *kvm, int fd, int gsi)
>  	init_waitqueue_func_entry(&irqfd->wait, irqfd_wakeup);
>  	init_poll_funcptr(&irqfd->pt, irqfd_ptable_queue_proc);
>  
> +	spin_lock_irq(&kvm->irqfds.lock);
> +
> +	ret = 0;
> +	list_for_each_entry(tmp, &kvm->irqfds.items, list) {
> +		if (irqfd->eventfd != tmp->eventfd)
> +			continue;
> +		/* This fd is used for another irq already. */
> +		ret = -EBUSY;
> +		spin_unlock_irq(&kvm->irqfds.lock);
> +		goto fail;
> +	}
> +
>  	events = file->f_op->poll(file, &irqfd->pt);
>  
> -	spin_lock_irq(&kvm->irqfds.lock);
>  	list_add_tail(&irqfd->list, &kvm->irqfds.items);
>  	spin_unlock_irq(&kvm->irqfds.lock);
>  

Re: [PATCH] kvm: only allow one gsi per fd

[Avi Kivity]

On 01/13/2010 06:58 PM, Michael S. Tsirkin wrote:
> Looks like repeatedly binding same fd to multiple gsi's with irqfd can
> use up a ton of kernel memory for irqfd structures.
>
> A simple fix is to allow each fd to only trigger one gsi: triggering a
> srorm of interrupts in guest is likely useless anyway, and we can do it
> by binding a single gsi to many interrupts if we really want to.
>    

Applied and queued, thanks.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] kvm: only allow one gsi per fd

[Michael S. Tsirkin]

On Sun, Jan 17, 2010 at 02:40:13PM +0200, Avi Kivity wrote:
> On 01/13/2010 06:58 PM, Michael S. Tsirkin wrote:
>> Looks like repeatedly binding same fd to multiple gsi's with irqfd can
>> use up a ton of kernel memory for irqfd structures.
>>
>> A simple fix is to allow each fd to only trigger one gsi: triggering a
>> srorm of interrupts in guest is likely useless anyway, and we can do it
>> by binding a single gsi to many interrupts if we really want to.
>>    
>
> Applied and queued, thanks.

I'm guessing we need this for 2.6.32 stable and 2.6.33 as well?

> -- 
> error compiling committee.c: too many arguments to function

Re: [PATCH] kvm: only allow one gsi per fd

[Avi Kivity]

On 01/20/2010 01:36 PM, Michael S. Tsirkin wrote:
> On Sun, Jan 17, 2010 at 02:40:13PM +0200, Avi Kivity wrote:
>    
>> On 01/13/2010 06:58 PM, Michael S. Tsirkin wrote:
>>      
>>> Looks like repeatedly binding same fd to multiple gsi's with irqfd can
>>> use up a ton of kernel memory for irqfd structures.
>>>
>>> A simple fix is to allow each fd to only trigger one gsi: triggering a
>>> srorm of interrupts in guest is likely useless anyway, and we can do it
>>> by binding a single gsi to many interrupts if we really want to.
>>>
>>>        
>> Applied and queued, thanks.
>>      
> I'm guessing we need this for 2.6.32 stable and 2.6.33 as well?
>    

That's what 'queued' means.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] kvm: only allow one gsi per fd

[Michael S. Tsirkin]

On Wed, Jan 20, 2010 at 01:52:00PM +0200, Avi Kivity wrote:
> On 01/20/2010 01:36 PM, Michael S. Tsirkin wrote:
>> On Sun, Jan 17, 2010 at 02:40:13PM +0200, Avi Kivity wrote:
>>    
>>> On 01/13/2010 06:58 PM, Michael S. Tsirkin wrote:
>>>      
>>>> Looks like repeatedly binding same fd to multiple gsi's with irqfd can
>>>> use up a ton of kernel memory for irqfd structures.
>>>>
>>>> A simple fix is to allow each fd to only trigger one gsi: triggering a
>>>> srorm of interrupts in guest is likely useless anyway, and we can do it
>>>> by binding a single gsi to many interrupts if we really want to.
>>>>
>>>>        
>>> Applied and queued, thanks.
>>>      
>> I'm guessing we need this for 2.6.32 stable and 2.6.33 as well?
>>    
>
> That's what 'queued' means.

Aha. Which git branch is used for these?

> -- 
> error compiling committee.c: too many arguments to function

Re: [PATCH] kvm: only allow one gsi per fd

[Avi Kivity]

On 01/20/2010 01:59 PM, Michael S. Tsirkin wrote:
>
>    
>>> I'm guessing we need this for 2.6.32 stable and 2.6.33 as well?
>>>
>>>        
>> That's what 'queued' means.
>>      
> Aha. Which git branch is used for these?
>
>    

kvm-updates/2.6.33.  Cc: stable@kernel.org means it will get 
auto-submitted to 2.6.32.

-- 
error compiling committee.c: too many arguments to function