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 <[email protected]>
---
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
>>> On 1/13/2010 at 11:58 AM, in message <[email protected]>,
"Michael S. Tsirkin" <[email protected]> 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 <[email protected]>
Seems reasonable to me.
Acked-by: Gregory Haskins <[email protected]>
> ---
>
> 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);
>
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
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
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
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
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: [email protected] means it will get
auto-submitted to 2.6.32.
--
error compiling committee.c: too many arguments to function