[Qemu-devel] [PATCH 0/5] s390x/kvm: avoid synchronize_rcu's in kernel

[Qemu-devel] [PATCH 0/5] s390x/kvm: avoid synchronize_rcu's in kernel

[Jens Freimann]

Whenever we call an ioctl from a wrong vcpu thread, the next ioctl will be
painfully slow because a synchronize_rcu thread has to be performed, therefore
involving all vcpu.

This patch series forces most ioctls to run on the associated vcpu.
It speeds up all start/restart/reset operations involving cpus drastically.


David Hildenbrand (5):
  kvm: run cpu state synchronization on target vcpu thread
  s390x/kvm: run guest triggered resets on the target vcpu thread
  s390x/kvm: execute sigp orders on the target vcpu thread
  s390x/kvm: execute "system reset" cpu resets on the vcpu thread
  s390x/kvm: execute the first cpu reset on the vcpu thread

 kvm-all.c                  | 18 ++++++++++++++++--
 target-s390x/cpu.c         |  6 +++++-
 target-s390x/cpu.h         | 15 +++++++++++++++
 target-s390x/kvm.c         | 23 ++++++++++++++++-------
 target-s390x/misc_helper.c | 30 ++++++++----------------------
 5 files changed, 60 insertions(+), 32 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/5] kvm: run cpu state synchronization on target vcpu thread

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

As already done for kvm_cpu_synchronize_state(), let's trigger
kvm_arch_put_registers() via run_on_cpu() for kvm_cpu_synchronize_post_reset()
and kvm_cpu_synchronize_post_init().

This way, we make sure that the register synchronizing ioctls are
called from the proper vcpu thread; this avoids calls to
synchronize_rcu() in the kernel.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

---
 kvm-all.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 1402f4f..b240bf8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1669,18 +1669,32 @@ void kvm_cpu_synchronize_state(CPUState *cpu)
     }
 }
 
-void kvm_cpu_synchronize_post_reset(CPUState *cpu)
+static void do_kvm_cpu_synchronize_post_reset(void *arg)
 {
+    CPUState *cpu = arg;
+
     kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE);
     cpu->kvm_vcpu_dirty = false;
 }
 
-void kvm_cpu_synchronize_post_init(CPUState *cpu)
+void kvm_cpu_synchronize_post_reset(CPUState *cpu)
+{
+    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, cpu);
+}
+
+static void do_kvm_cpu_synchronize_post_init(void *arg)
 {
+    CPUState *cpu = arg;
+
     kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE);
     cpu->kvm_vcpu_dirty = false;
 }
 
+void kvm_cpu_synchronize_post_init(CPUState *cpu)
+{
+    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, cpu);
+}
+
 int kvm_cpu_exec(CPUState *cpu)
 {
     struct kvm_run *run = cpu->kvm_run;
-- 
1.9.3

[Qemu-devel] [PATCH 2/5] s390x/kvm: run guest triggered resets on the target vcpu thread

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

Currently, load_normal_reset() and modified_clear_reset() as triggered
by a guest vcpu will initiate cpu resets on the current vcpu thread for
all cpus. The reset should happen on the individual vcpu thread
instead, so let's use run_on_cpu() for this.

This avoids calls to synchronize_rcu() in the kernel.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/cpu.h         | 15 +++++++++++++++
 target-s390x/misc_helper.c | 30 ++++++++----------------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index b13761d..17a3df4 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -353,6 +353,21 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb)
 /* Base/displacement are at the same locations. */
 #define decode_basedisp_rs decode_basedisp_s
 
+/* helper functions for run_on_cpu() */
+static inline void s390_do_cpu_reset(void *arg)
+{
+    CPUState *cs = arg;
+    S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
+
+    scc->cpu_reset(cs);
+}
+static inline void s390_do_cpu_full_reset(void *arg)
+{
+    CPUState *cs = arg;
+
+    cpu_reset(cs);
+}
+
 void s390x_tod_timer(void *opaque);
 void s390x_cpu_timer(void *opaque);
 
diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index 0b62582..ef9758a 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -114,33 +114,16 @@ uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
 }
 
 #ifndef CONFIG_USER_ONLY
-static void cpu_reset_all(void)
-{
-    CPUState *cs;
-    S390CPUClass *scc;
-
-    CPU_FOREACH(cs) {
-        scc = S390_CPU_GET_CLASS(cs);
-        scc->cpu_reset(cs);
-    }
-}
-
-static void cpu_full_reset_all(void)
-{
-    CPUState *cpu;
-
-    CPU_FOREACH(cpu) {
-        cpu_reset(cpu);
-    }
-}
-
 static int modified_clear_reset(S390CPU *cpu)
 {
     S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
+    CPUState *t;
 
     pause_all_vcpus();
     cpu_synchronize_all_states();
-    cpu_full_reset_all();
+    CPU_FOREACH(t) {
+        run_on_cpu(t, s390_do_cpu_full_reset, t);
+    }
     cmma_reset(cpu);
     io_subsystem_reset();
     scc->load_normal(CPU(cpu));
@@ -152,10 +135,13 @@ static int modified_clear_reset(S390CPU *cpu)
 static int load_normal_reset(S390CPU *cpu)
 {
     S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
+    CPUState *t;
 
     pause_all_vcpus();
     cpu_synchronize_all_states();
-    cpu_reset_all();
+    CPU_FOREACH(t) {
+        run_on_cpu(t, s390_do_cpu_reset, t);
+    }
     cmma_reset(cpu);
     io_subsystem_reset();
     scc->initial_cpu_reset(CPU(cpu));
-- 
1.9.3

[Qemu-devel] [PATCH 3/5] s390x/kvm: execute sigp orders on the target vcpu thread

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

All sigp orders that can result in ioctls on the target vcpu should be executed
on the associated vcpu thread.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/kvm.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index a32d91a..421ff43 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -916,23 +916,30 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
     return r;
 }
 
-static int kvm_s390_cpu_start(S390CPU *cpu)
+static void sigp_cpu_start(void *arg)
 {
+    CPUState *cs = arg;
+    S390CPU *cpu = S390_CPU(cs);
+
     s390_add_running_cpu(cpu);
-    qemu_cpu_kick(CPU(cpu));
     DPRINTF("DONE: KVM cpu start: %p\n", &cpu->env);
-    return 0;
 }
 
-int kvm_s390_cpu_restart(S390CPU *cpu)
+static void sigp_cpu_restart(void *arg)
 {
+    CPUState *cs = arg;
+    S390CPU *cpu = S390_CPU(cs);
     struct kvm_s390_irq irq = {
         .type = KVM_S390_RESTART,
     };
 
     kvm_s390_vcpu_interrupt(cpu, &irq);
     s390_add_running_cpu(cpu);
-    qemu_cpu_kick(CPU(cpu));
+}
+
+int kvm_s390_cpu_restart(S390CPU *cpu)
+{
+    run_on_cpu(CPU(cpu), sigp_cpu_restart, CPU(cpu));
     DPRINTF("DONE: KVM cpu restart: %p\n", &cpu->env);
     return 0;
 }
@@ -980,10 +987,12 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
 
     switch (order_code) {
     case SIGP_START:
-        cc = kvm_s390_cpu_start(target_cpu);
+        run_on_cpu(CPU(target_cpu), sigp_cpu_start, CPU(target_cpu));
+        cc = 0;
         break;
     case SIGP_RESTART:
-        cc = kvm_s390_cpu_restart(target_cpu);
+        run_on_cpu(CPU(target_cpu), sigp_cpu_restart, CPU(target_cpu));
+        cc = 0;
         break;
     case SIGP_SET_ARCH:
         *statusreg &= 0xffffffff00000000UL;
-- 
1.9.3

[Qemu-devel] [PATCH 4/5] s390x/kvm: execute "system reset" cpu resets on the vcpu thread

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

Let's execute resets triggered by qemu system resets on the target vcpu thread.
This will avoid synchronize_rcu's in the kernel.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index c3082b7..4633282 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -165,7 +165,7 @@ static void s390_cpu_machine_reset_cb(void *opaque)
 {
     S390CPU *cpu = opaque;
 
-    cpu_reset(CPU(cpu));
+    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, CPU(cpu));
 }
 #endif
 
-- 
1.9.3

[Qemu-devel] [PATCH 5/5] s390x/kvm: execute the first cpu reset on the vcpu thread

[Jens Freimann]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

As all full cpu resets currently call into the kernel to do initial cpu reset,
let's run this reset (triggered by cpu_s390x_init()) on the proper vcpu thread.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 target-s390x/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 4633282..505a2fa 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -175,7 +175,11 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     S390CPUClass *scc = S390_CPU_GET_CLASS(dev);
 
     qemu_init_vcpu(cs);
+#if !defined(CONFIG_USER_ONLY)
+    run_on_cpu(cs, s390_do_cpu_full_reset, cs);
+#else
     cpu_reset(cs);
+#endif
 
     scc->parent_realize(dev, errp);
 }
-- 
1.9.3

Re: [Qemu-devel] [PATCH 0/5] s390x/kvm: avoid synchronize_rcu's in kernel

[Christian Borntraeger]

On 28/08/14 13:58, Jens Freimann wrote:
> Whenever we call an ioctl from a wrong vcpu thread, the next ioctl will be
> painfully slow because a synchronize_rcu thread has to be performed, therefore
> involving all vcpu.
> 
> This patch series forces most ioctls to run on the associated vcpu.
> It speeds up all start/restart/reset operations involving cpus drastically.
> 
> 
> David Hildenbrand (5):
>   kvm: run cpu state synchronization on target vcpu thread
>   s390x/kvm: run guest triggered resets on the target vcpu thread
>   s390x/kvm: execute sigp orders on the target vcpu thread
>   s390x/kvm: execute "system reset" cpu resets on the vcpu thread
>   s390x/kvm: execute the first cpu reset on the vcpu thread
> 
>  kvm-all.c                  | 18 ++++++++++++++++--
>  target-s390x/cpu.c         |  6 +++++-
>  target-s390x/cpu.h         | 15 +++++++++++++++
>  target-s390x/kvm.c         | 23 ++++++++++++++++-------
>  target-s390x/misc_helper.c | 30 ++++++++----------------------
>  5 files changed, 60 insertions(+), 32 deletions(-)
> 
Thanks.

Applied 2-5 to https://github.com/borntraeger/qemu.git s390-next.
Patch 1 will be taken care of in Paolos kvm tree.

I will wait some more days before sending the pull request to give others a chance for feedback.

Christian