[Qemu-devel] [PATCH 0/2] TLS abstraction layer for thread-local cpu_single_env on Linux

[Qemu-devel] [PATCH 0/2] TLS abstraction layer for thread-local cpu_single_env on Linux

[Peter Maydell]

These patches add enough of the TLS abstraction layer to allow us
to make cpu_single_env thread-local on Linux systems. This fixes
the regression described in bug 823902 for the 1.0 release; we
can add the Win32 and POSIX implementations later.          

I haven't included Paolo's "Prepare Windows port for thread-local
cpu_single_env" patch -- it would be safe to do so but it isn't
necessary until we actually implement TLS for Win32.     

Paolo Bonzini (1):
  Make cpu_single_env thread-local

Peter Maydell (1):
  qemu-tls.h: Add abstraction layer for TLS variables

 cpu-all.h          |    4 +++-
 darwin-user/main.c |    2 --
 exec.c             |    2 +-
 qemu-tls.h         |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 55 insertions(+), 4 deletions(-)
 create mode 100644 qemu-tls.h

[Qemu-devel] [PATCH 1/2] qemu-tls.h: Add abstraction layer for TLS variables

[Peter Maydell]

Add an abstraction layer for defining and using thread-local
variables. For the moment this is implemented only for Linux,
which means they can only be used in restricted circumstances.
The abstraction layer allows us to add POSIX and Win32 support
later.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 qemu-tls.h |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)
 create mode 100644 qemu-tls.h

diff --git a/qemu-tls.h b/qemu-tls.h
new file mode 100644
index 0000000..79f9417
--- /dev/null
+++ b/qemu-tls.h
@@ -0,0 +1,51 @@
+/*
+ * Abstraction layer for defining and using TLS variables
+ *
+ * Copyright (c) 2011 Red Hat, Inc, Linaro Limited
+ *
+ * Authors:
+ *  Paolo Bonzini <pbonzini@us.ibm.com>
+ *  Peter Maydell <peter.maydell@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QEMU_TLS_GCC_H
+#define QEMU_TLS_GCC_H
+
+/* Per-thread variables. Note that we only have implementations
+ * which are really thread-local on Linux; the dummy implementations
+ * define plain global variables.
+ *
+ * This means that for the moment use should be restricted to
+ * per-VCPU variables, which are OK because:
+ *  - the only -user mode supporting multiple VCPU threads is linux-user
+ *  - TCG system mode is single-threaded regarding VCPUs
+ *  - KVM system mode is multi-threaded but limited to Linux
+ *
+ * TODO: proper implementations via Win32 .tls sections and
+ * POSIX pthread_getspecific.
+ */
+#ifdef __linux__
+#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x)
+#define DEFINE_TLS(type, x)  __thread __typeof__(type) tls__##x
+#define get_tls(x)           tls__##x
+#else
+/* Dummy implementations which define plain global variables */
+#define DECLARE_TLS(type, x) extern DEFINE_TLS(type, x)
+#define DEFINE_TLS(type, x)  __typeof__(type) tls__##x
+#define get_tls(x)           tls__##x
+#endif
+
+#endif
-- 
1.7.1

[Qemu-devel] [PATCH 2/2] Make cpu_single_env thread-local

[Peter Maydell]

From: Paolo Bonzini <pbonzini@redhat.com>

Make cpu_single_env thread-local. This fixes a regression
in handling of multi-threaded programs in linux-user mode
(bug 823902).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-all.h          |    4 +++-
 darwin-user/main.c |    2 --
 exec.c             |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 42a5fa0..e37ebfc 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -20,6 +20,7 @@
 #define CPU_ALL_H
 
 #include "qemu-common.h"
+#include "qemu-tls.h"
 #include "cpu-common.h"
 
 /* some important defines:
@@ -334,7 +335,8 @@ void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
 void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 extern CPUState *first_cpu;
-extern CPUState *cpu_single_env;
+DECLARE_TLS(CPUState *,tls_cpu_single_env);
+#define cpu_single_env get_tls(tls_cpu_single_env)
 
 /* Flags for use in ENV->INTERRUPT_PENDING.
 
diff --git a/darwin-user/main.c b/darwin-user/main.c
index 1a881a0..c0f14f8 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -729,8 +729,6 @@ static void usage(void)
 
 /* XXX: currently only used for async signals (see signal.c) */
 CPUState *global_env;
-/* used only if single thread */
-CPUState *cpu_single_env = NULL;
 
 /* used to free thread contexts */
 TaskState *first_task_state;
diff --git a/exec.c b/exec.c
index 9dc4edb..ea4e8de 100644
--- a/exec.c
+++ b/exec.c
@@ -120,7 +120,7 @@ static MemoryRegion *system_io;
 CPUState *first_cpu;
 /* current CPU in the current thread. It is only valid inside
    cpu_exec() */
-CPUState *cpu_single_env;
+DEFINE_TLS(CPUState *,tls_cpu_single_env);
 /* 0 = Do not count executed instructions.
    1 = Precise instruction counting.
    2 = Adaptive rate instruction counting.  */
-- 
1.7.1

Re: [Qemu-devel] [PATCH 2/2] Make cpu_single_env thread-local

[Andreas Färber]

Am 26.10.2011 18:07, schrieb Peter Maydell:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Make cpu_single_env thread-local. This fixes a regression
> in handling of multi-threaded programs in linux-user mode
> (bug 823902).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpu-all.h          |    4 +++-
>  darwin-user/main.c |    2 --
>  exec.c             |    2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/cpu-all.h b/cpu-all.h
> index 42a5fa0..e37ebfc 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -20,6 +20,7 @@
>  #define CPU_ALL_H
>  
>  #include "qemu-common.h"
> +#include "qemu-tls.h"
>  #include "cpu-common.h"
>  
>  /* some important defines:
> @@ -334,7 +335,8 @@ void cpu_dump_statistics(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
>  void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
>      GCC_FMT_ATTR(2, 3);
>  extern CPUState *first_cpu;
> -extern CPUState *cpu_single_env;
> +DECLARE_TLS(CPUState *,tls_cpu_single_env);

So this effectively becomes tls__tls_cpu_single_env iiuc.
For compile-checking for missed conversions,

DECLARE_TLS(CPUState *,cpu_single_env);

should do, no?

> diff --git a/darwin-user/main.c b/darwin-user/main.c
> index 1a881a0..c0f14f8 100644
> --- a/darwin-user/main.c
> +++ b/darwin-user/main.c
> @@ -729,8 +729,6 @@ static void usage(void)
>  
>  /* XXX: currently only used for async signals (see signal.c) */
>  CPUState *global_env;
> -/* used only if single thread */
> -CPUState *cpu_single_env = NULL;
>  
>  /* used to free thread contexts */
>  TaskState *first_task_state;

This is not making anything local, it's apparently removing an unused
variable. Can we then have that in a patch saying so, please?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 2/2] Make cpu_single_env thread-local

[Peter Maydell]

On 26 October 2011 17:20, Andreas Färber <afaerber@suse.de> wrote:
>> +DECLARE_TLS(CPUState *,tls_cpu_single_env);
>
> So this effectively becomes tls__tls_cpu_single_env iiuc.
> For compile-checking for missed conversions,
>
> DECLARE_TLS(CPUState *,cpu_single_env);
>
> should do, no?

Yes. (That does impose the silent requirement that DEFINE_TLS(type, x)
is defined such that it only uses its parameter 'x' with the ##
concatenation operator, though, otherwise when exec.c invokes
DEFINE_TLS(CPUState *,cpu_single_env) we'd get an unwanted extra
expansion. I don't know if we care about that.)

>> diff --git a/darwin-user/main.c b/darwin-user/main.c
>> index 1a881a0..c0f14f8 100644
>> --- a/darwin-user/main.c
>> +++ b/darwin-user/main.c
>> @@ -729,8 +729,6 @@ static void usage(void)
>>
>>  /* XXX: currently only used for async signals (see signal.c) */
>>  CPUState *global_env;
>> -/* used only if single thread */
>> -CPUState *cpu_single_env = NULL;
>>
>>  /* used to free thread contexts */
>>  TaskState *first_task_state;
>
> This is not making anything local, it's apparently removing an unused
> variable. Can we then have that in a patch saying so, please?

Sure.

-- PMM

Re: [Qemu-devel] [PATCH 1/2] qemu-tls.h: Add abstraction layer for TLS variables

[Paolo Bonzini]

On 10/26/2011 06:07 PM, Peter Maydell wrote:
> + * Authors:
> + *  Paolo Bonzini<pbonzini@us.ibm.com>

Wrong email address,

> + *  Peter Maydell<peter.maydell@linaro.org>

otherwise looks fine.

Paolo

Re: [Qemu-devel] [PATCH 1/2] qemu-tls.h: Add abstraction layer for TLS variables

[Peter Maydell]

On 26 October 2011 18:08, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 10/26/2011 06:07 PM, Peter Maydell wrote:
>>
>> + * Authors:
>> + *  Paolo Bonzini<pbonzini@us.ibm.com>
>
> Wrong email address,

Rats, I talked to you about this and then must have failed
to actually make the edit! Oh well, fixed for next version.

-- PMM