> > Based on discussions in:
> > http://lists.gnu.org/archive/html/qemu-devel/2013-11/threads.html#03322
> >
> > About KVM_SET_GSI_ROUTING ioctl, I tested changing RCU to SRCU, but
> unfortunately
> > it looks like SRCU's grace period is no better than RCU.
> 
> Really?  This is not what Christian Borntraeger claimed at
> http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/118083 -- in
> fact, Andrew Theurer is currently testing a variant of that patch and I
> was going to post it for 3.16.
> 
> Have you tried synchronize_srcu_expedited?  Unlike the RCU variant, you
> can use this function.
> 
Yes, previously I was using synchronize_srcu, which is not good. When I 
changed it to synchronize_srcu_expedited, grace period delay is much better 
than synchronize_srcu. Though in our tests, we can still see some impact 
of KVM_SET_GSI_ROUTING ioctl.

Our testing scenario is like this. In VM we run a script that sets smp_affinity 
for each IRQ every 0.5s (this leads QEMU to do KVM_SET_GSI_ROUTING ioctl).
Outside the VM we ping that VM.

Without patches, ping time can jump from 0.3ms to 2ms-30ms. With 
synchronize_srcu 
patch, ping time is worse. With synchronize_srcu_expedited patch, ping time is 
overall good, though sometimes ping time jump to 1ms-3ms.

With following raw patch, ping time is like call_rcu patch, that not influenced 
by setting IRQ affinity, keeps 0.3ms, and there is no vulnerability, frequent 
intermidiate KVM_SET_GSI_ROUTING settings are just skipped, and always the 
newest 
setting would take effect.

Would you think this patch is acceptable or has problem? Thanks in advance.

diff -urp kvm_kmod/include/linux/kvm_host.h 
kvm_kmod_new//include/linux/kvm_host.h
--- kvm_kmod/include/linux/kvm_host.h   2014-03-10 14:08:28.000000000 +0000
+++ kvm_kmod_new//include/linux/kvm_host.h      2014-03-26 15:07:48.000000000 
+0000
@@ -337,6 +337,12 @@ struct kvm {
 
        struct mutex irq_lock;
 #ifdef CONFIG_HAVE_KVM_IRQCHIP
+       struct task_struct *kthread;
+       wait_queue_head_t wq;
+       struct mutex irq_routing_lock;
+       struct kvm_irq_routing to_update_routing;
+       struct kvm_irq_routing_entry *to_update_entries;
+       atomic_t have_new;
        /*
         * Update side is protected by irq_lock and,
         * if configured, irqfds.lock.
diff -urp kvm_kmod/x86/assigned-dev.c kvm_kmod_new//x86/assigned-dev.c
--- kvm_kmod/x86/assigned-dev.c 2014-03-10 14:08:28.000000000 +0000
+++ kvm_kmod_new//x86/assigned-dev.c    2014-03-26 15:22:33.000000000 +0000
@@ -1056,12 +1056,23 @@ long kvm_vm_ioctl_assigned_device(struct
                r = -EFAULT;
                urouting = argp;
                if (copy_from_user(entries, urouting->entries,
-                                  routing.nr * sizeof(*entries)))
-                       goto out_free_irq_routing;
-               r = kvm_set_irq_routing(kvm, entries, routing.nr,
-                                       routing.flags);
-       out_free_irq_routing:
-               vfree(entries);
+                                  routing.nr * sizeof(*entries))) {
+                       vfree(entries);
+                       break;
+               }
+
+               mutex_lock(&kvm->irq_routing_lock);
+               if (kvm->to_update_entries) {
+                       vfree(kvm->to_update_entries);
+                       kvm->to_update_entries = NULL;
+               }
+               kvm->to_update_routing = routing;
+               kvm->to_update_entries = entries;
+               atomic_set(&kvm->have_new, 1);
+               mutex_unlock(&kvm->irq_routing_lock);
+
+               wake_up(&kvm->wq);
+               r = 0;  /* parameter validity or memory alloc avalibity should 
be checked before return */
                break;
        }
 #endif /* KVM_CAP_IRQ_ROUTING */
diff -urp kvm_kmod/x86/kvm_main.c kvm_kmod_new//x86/kvm_main.c
--- kvm_kmod/x86/kvm_main.c     2014-03-10 14:08:28.000000000 +0000
+++ kvm_kmod_new//x86/kvm_main.c        2014-03-26 15:27:02.000000000 +0000
@@ -83,6 +83,7 @@
 #include <linux/slab.h>
 #include <linux/sort.h>
 #include <linux/bsearch.h>
+#include <linux/kthread.h>
 
 #include <asm/processor.h>
 #include <asm/io.h>
@@ -501,6 +502,49 @@ static void kvm_init_memslots_id(struct
                slots->id_to_index[i] = slots->memslots[i].id = i;
 }
 
+static int do_irq_routing_table_update(struct kvm *kvm)
+{
+       struct kvm_irq_routing_entry *entries;
+       unsigned nr;
+       unsigned flags;
+
+       mutex_lock(&kvm->irq_routing_lock);
+       BUG_ON(!kvm->to_update_entries);
+
+       nr = kvm->to_update_routing.nr;
+       flags = kvm->to_update_routing.flags;
+       entries = vmalloc(nr * sizeof(*entries));
+       if (!entries) {
+               mutex_unlock(&kvm->irq_routing_lock);
+               return 0;
+       }
+       /* this memcpy can be eliminated by doing set in mutex_lock and doing 
synchronize_rcu outside */
+       memcpy(entries, kvm->to_update_entries, nr * sizeof(*entries));
+
+       atomic_set(&kvm->have_new, 0);
+       mutex_unlock(&kvm->irq_routing_lock);
+
+       kvm_set_irq_routing(kvm, entries, nr, flags);
+
+       return 0;
+}
+
+static int do_irq_routing_rcu(void *data)
+{
+       struct kvm *kvm = (struct kvm *)data;
+
+       while (1) {
+               wait_event_interruptible(kvm->wq,
+                       atomic_read(&kvm->have_new) || kthread_should_stop());
+
+               if (kthread_should_stop())
+                       break;
+
+               do_irq_routing_table_update(kvm);
+       }
+
+       return 0;
+}
+
 static struct kvm *kvm_create_vm(unsigned long type)
 {
        int r, i;
@@ -529,6 +573,12 @@ static struct kvm *kvm_create_vm(unsigne
        kvm_init_memslots_id(kvm);
        if (init_srcu_struct(&kvm->srcu))
                goto out_err_nosrcu;
+
+       atomic_set(&kvm->have_new, 0);
+       init_waitqueue_head(&kvm->wq);
+       mutex_init(&kvm->irq_routing_lock);
+       kvm->kthread = kthread_run(do_irq_routing_rcu, kvm, "irq_routing");
+
        for (i = 0; i < KVM_NR_BUSES; i++) {
                kvm->buses[i] = kzalloc(sizeof(struct kvm_io_bus),
                                        GFP_KERNEL);
@@ -635,6 +685,11 @@ static void kvm_destroy_vm(struct kvm *k
        list_del(&kvm->vm_list);
        raw_spin_unlock(&kvm_lock);
        kvm_free_irq_routing(kvm);
+
+       kthread_stop(kvm->kthread);
+       if (kvm->to_update_entries)
+               vfree(kvm->to_update_entries);
+
        for (i = 0; i < KVM_NR_BUSES; i++)
                kvm_io_bus_destroy(kvm->buses[i]);
        kvm_coalesced_mmio_free(kvm);


Best regards,
-Gonglei
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to