[Qemu-devel] [PATCH 0/3] Release SMP restriction on Windows host

[Qemu-devel] [PATCH 0/3] Release SMP restriction on Windows host

[Fabien Chouteau]

As I already explained to this mailling list, we (AdaCore) use QEMU to run a
lot of short executable (~2 secs) on many different targets. In that
configuration the SMP restriction on Windows (QEMU runs only on 1 core) was not
efficient on our 16 cores servers.

Few months ago I asked on this mailing list if the restriction could be
removed. There was no clear answer because nobody was able to tell if it would
work. We decided to give it a try.

During the last months we only found one problem, it was a dead lock between
the TCG and IO thread, this is fixed by the second patch.

We also found that suspendThread() and resumeThread() were not synchronous on
SMP systems, so we added a busy loop to wait for effective suspension.

Since then, we run thousands of QEMU on our SMP Windows servers each day
without any problem. And of course we greatly improve the execution time of our
test-suites.

Fabien Chouteau (1):
  Release SMP restriction on Windows

Olivier Hainque (2):
  Check effective suspension of TCG thread
  Ensure good ordering of memory instruction in cpu_exec

 cpu-exec.c |    8 ++++++++
 cpus.c     |   24 ++++++++++++++++++++++--
 os-win32.c |   18 ------------------
 3 files changed, 30 insertions(+), 20 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/3] Check effective suspension of TCG thread

[Fabien Chouteau]

From: Olivier Hainque <hainque@adacore.com>

On multi-core systems, SuspendThread does not guaranty immediate thread
suspension. We add busy loop to wait for effective thread suspension
after call to ThreadSuspend().

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 cpus.c |   24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index e919dd7..97e9ab4 100644
--- a/cpus.c
+++ b/cpus.c
@@ -862,9 +862,29 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
     }
 #else /* _WIN32 */
     if (!qemu_cpu_is_self(cpu)) {
-        SuspendThread(cpu->hThread);
+        CONTEXT tcgContext;
+
+        if (SuspendThread(cpu->hThread) == (DWORD)-1) {
+            fprintf(stderr, "qemu:%s: GetLastError:%d\n", __func__,
+                    GetLastError());
+            exit(1);
+        }
+
+        /* On multi-core systems, we are not sure that the thread is actually
+         * suspended until we can get the context.
+         */
+        tcgContext.ContextFlags = CONTEXT_CONTROL;
+        while (GetThreadContext(cpu->hThread, &tcgContext) != 0) {
+            continue;
+        }
+
         cpu_signal(0);
-        ResumeThread(cpu->hThread);
+
+        if (ResumeThread(cpu->hThread) == (DWORD)-1) {
+            fprintf(stderr, "qemu:%s: GetLastError:%d\n", __func__,
+                    GetLastError());
+            exit(1);
+        }
     }
 #endif
 }
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/3] Ensure good ordering of memory instruction in cpu_exec

[Fabien Chouteau]

From: Olivier Hainque <hainque@adacore.com>

The IO thread, when it senses cpu_single_env == 0, expects exit_request
to be checked later on. A compiler scheduling constraint is not strong
enough to ensure this on modern architecture. A memory fence is needed
as well.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 cpu-exec.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/cpu-exec.c b/cpu-exec.c
index e74e556..aa8fa89 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -217,6 +217,14 @@ int cpu_exec(CPUArchState *env)
 
     cpu_single_env = env;
 
+    /* As long as cpu_single_env is null, up to the assignment just above,
+     * requests by other threads to exit the execution loop are expected to
+     * be issued using the exit_request global. We must make sure that our
+     * evaluation of the global value is performed past the cpu_single_env
+     * value transition point, which requires a memory barrier as well as
+     * an instruction scheduling constraint on modern architectures.  */
+    smp_mb();
+
     if (unlikely(exit_request)) {
         cpu->exit_request = 1;
     }
-- 
1.7.9.5

[Qemu-devel] [PATCH 3/3] Release SMP restriction on Windows

[Fabien Chouteau]

The previous patches make QEMU SMP safe on Windows, we can now release
the restriction.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 os-win32.c |   18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/os-win32.c b/os-win32.c
index 9673a81..c7f6b5c 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -69,25 +69,7 @@ static BOOL WINAPI qemu_ctrl_handler(DWORD type)
 
 void os_setup_early_signal_handling(void)
 {
-    /* Note: cpu_interrupt() is currently not SMP safe, so we force
-       QEMU to run on a single CPU */
-    HANDLE h;
-    DWORD_PTR mask, smask;
-    int i;
-
     SetConsoleCtrlHandler(qemu_ctrl_handler, TRUE);
-
-    h = GetCurrentProcess();
-    if (GetProcessAffinityMask(h, &mask, &smask)) {
-        for(i = 0; i < 32; i++) {
-            if (mask & (1 << i))
-                break;
-        }
-        if (i != 32) {
-            mask = 1 << i;
-            SetProcessAffinityMask(h, mask);
-        }
-    }
 }
 
 /* Look for support files in the same directory as the executable.  */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 0/3] Release SMP restriction on Windows host

[Paolo Bonzini]

Il 09/04/2013 18:06, Fabien Chouteau ha scritto:
> As I already explained to this mailling list, we (AdaCore) use QEMU to run a
> lot of short executable (~2 secs) on many different targets. In that
> configuration the SMP restriction on Windows (QEMU runs only on 1 core) was not
> efficient on our 16 cores servers.
> 
> Few months ago I asked on this mailing list if the restriction could be
> removed. There was no clear answer because nobody was able to tell if it would
> work. We decided to give it a try.
> 
> During the last months we only found one problem, it was a dead lock between
> the TCG and IO thread, this is fixed by the second patch.
> 
> We also found that suspendThread() and resumeThread() were not synchronous on
> SMP systems, so we added a busy loop to wait for effective suspension.
> 
> Since then, we run thousands of QEMU on our SMP Windows servers each day
> without any problem. And of course we greatly improve the execution time of our
> test-suites.

Awesome!  Thanks guys.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [Qemu-devel] [PATCH 0/3] Release SMP restriction on Windows host

[Stefan Weil]

On 09.04.2013 18:16, Paolo Bonzini wrote:
> Reviewed-by: Paolo Bonzini<pbonzini@redhat.com>

Do we need an additional Signed-off from the author (Olivier Hainque)?
Currently, only Fabien signed these patches.

I'm preparing a pull request and was not sure about this point.

Regards,

Stefan W.

Re: [Qemu-devel] [PATCH 0/3] Release SMP restriction on Windows host

[Paolo Bonzini]

Il 11/04/2013 20:57, Stefan Weil ha scritto:
> On 09.04.2013 18:16, Paolo Bonzini wrote:
>> Reviewed-by: Paolo Bonzini<pbonzini@redhat.com>
> 
> Do we need an additional Signed-off from the author (Olivier Hainque)?
> Currently, only Fabien signed these patches.

In theory yes, in practice they are colleagues so I think it's fine.

Paolo

Re: [Qemu-devel] [PATCH 0/3] Release SMP restriction on Windows host

[Olivier Hainque]

On Apr 12, 2013, at 08:40 , Paolo Bonzini <pbonzini@redhat.com> wrote:

>> Do we need an additional Signed-off from the author (Olivier Hainque)?
>> Currently, only Fabien signed these patches.
> 
> In theory yes, in practice they are colleagues so I think it's fine.

 We are colleagues indeed and there's no problem having the
 patches signed by Fabien only.

   [Qemu-devel] [PATCH 1/3] Check effective suspension of TCG thread

 Was essentially written by Fabien anyway :)


 If you'd rather have a distinct signoff for

   [Qemu-devel] [PATCH 2/3] Ensure good ordering of memory instruction cpu_exec
 
 ... fine with me of course:

 Signed-off-by: Olivier Hainque <hainque@adacore.com>

 Thanks!

 Olivier