* [PATCH] kvm: only allow one gsi per fd
@ 2010-01-13 16:58 Michael S. Tsirkin
2010-01-13 18:22 ` Gregory Haskins
2010-01-17 12:40 ` Avi Kivity
0 siblings, 2 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2010-01-13 16:58 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti, Michael S. Tsirkin, Gregory Haskins,
Gleb Natapov, Julia Lawall, kvm, linux-kernel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: only allow one gsi per fd
2010-01-13 16:58 [PATCH] kvm: only allow one gsi per fd Michael S. Tsirkin
@ 2010-01-13 18:22 ` Gregory Haskins
2010-01-17 12:40 ` Avi Kivity
1 sibling, 0 replies; 7+ messages in thread
From: Gregory Haskins @ 2010-01-13 18:22 UTC (permalink / raw)
To: Julia Lawall, Avi Kivity, Gleb Natapov, Michael S. Tsirkin,
Marcelo Tosatti, kvm, linux-kernel
>>> 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);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: only allow one gsi per fd
2010-01-13 16:58 [PATCH] kvm: only allow one gsi per fd Michael S. Tsirkin
2010-01-13 18:22 ` Gregory Haskins
@ 2010-01-17 12:40 ` Avi Kivity
2010-01-20 11:36 ` Michael S. Tsirkin
1 sibling, 1 reply; 7+ messages in thread
From: Avi Kivity @ 2010-01-17 12:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Marcelo Tosatti, Gregory Haskins, Gleb Natapov, Julia Lawall,
kvm, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: only allow one gsi per fd
2010-01-17 12:40 ` Avi Kivity
@ 2010-01-20 11:36 ` Michael S. Tsirkin
2010-01-20 11:52 ` Avi Kivity
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2010-01-20 11:36 UTC (permalink / raw)
To: Avi Kivity
Cc: Marcelo Tosatti, Gregory Haskins, Gleb Natapov, Julia Lawall,
kvm, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: only allow one gsi per fd
2010-01-20 11:36 ` Michael S. Tsirkin
@ 2010-01-20 11:52 ` Avi Kivity
2010-01-20 11:59 ` Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Avi Kivity @ 2010-01-20 11:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Marcelo Tosatti, Gregory Haskins, Gleb Natapov, Julia Lawall,
kvm, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: only allow one gsi per fd
2010-01-20 11:52 ` Avi Kivity
@ 2010-01-20 11:59 ` Michael S. Tsirkin
2010-01-20 12:11 ` Avi Kivity
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2010-01-20 11:59 UTC (permalink / raw)
To: Avi Kivity
Cc: Marcelo Tosatti, Gregory Haskins, Gleb Natapov, Julia Lawall,
kvm, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm: only allow one gsi per fd
2010-01-20 11:59 ` Michael S. Tsirkin
@ 2010-01-20 12:11 ` Avi Kivity
0 siblings, 0 replies; 7+ messages in thread
From: Avi Kivity @ 2010-01-20 12:11 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Marcelo Tosatti, Gregory Haskins, Gleb Natapov, Julia Lawall,
kvm, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-01-20 12:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-13 16:58 [PATCH] kvm: only allow one gsi per fd Michael S. Tsirkin
2010-01-13 18:22 ` Gregory Haskins
2010-01-17 12:40 ` Avi Kivity
2010-01-20 11:36 ` Michael S. Tsirkin
2010-01-20 11:52 ` Avi Kivity
2010-01-20 11:59 ` Michael S. Tsirkin
2010-01-20 12:11 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).