Skip to contentSkip to conversation

kvm: only allow one gsi per fd

PATCH In review Michael S. Tsirkin 3 participants 7 messages Jan 13, 2010 – Jan 20, 2010 KVM

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kvm: only allow one gsi per fd

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.

---

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(-)
virt/kvm/eventfd.c +13 -2
index 30f70fd..62e4cd9 100644
@@ -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);
 
Signature · 1 line
1.6.6.144.g5c3af
Acked-by on v1 1/1
#1 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line

On 1/13/2010 at 11:58 AM, in message <[email protected]>,

Quoted 8 lines · 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.

Seems reasonable to me.

Quoted 43 lines

---

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(-)
virt/kvm/eventfd.c +1 -1
index 30f70fd..62e4cd9 100644
@@ -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;
virt/kvm/eventfd.c +12 -1
 	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);
 
#2 lore raw
In reply to Michael S. Tsirkin
Quoted 7 lines · 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.

Applied and queued, thanks.

Signature · 1 line
error compiling committee.c: too many arguments to function
#3 lore raw
Quoted 10 lines · Avi Kivity
Quoted 7 lines · 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.

Applied and queued, thanks.

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

Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
#4 lore raw
Quoted 7 lines · Michael S. Tsirkin
Quoted 5 lines · Avi Kivity
Quoted 3 lines · 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.

Quoted 9 lines
Quoted 7 lines
Quoted 5 lines

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#5 lore raw
Quoted 13 lines · Avi Kivity
Quoted 12 lines · Michael S. Tsirkin
Quoted 10 lines · Avi Kivity
Quoted 8 lines · 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.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

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?

Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
#6 lore raw
Quoted 10 lines · Michael S. Tsirkin
Quoted 7 lines
Quoted 5 lines

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: [email protected] means it will get auto-submitted to 2.6.32.

Signature · 1 line
error compiling committee.c: too many arguments to function