> > 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
