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

[Qemu-devel] [PATCH v2 0/3] 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.

Changes v1->v2:
 * fix Paolo's email address
 * split the darwin-user change out into a separate patch
 * drop the 'tls_' prefix from the cpu_single_env tls var name

Paolo Bonzini (2):
  darwin-user/main.c: Drop unused cpu_single_env definition
  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 v2 1/3] 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..d96a159
--- /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@redhat.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

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

[Andreas Färber]

Am 27.10.2011 13:37, schrieb 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.
> 

[Paolo's SoB missing]

> 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..d96a159
> --- /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

The concatenation looks kind of funny. ;)

> + *
> + * Authors:
> + *  Paolo Bonzini <pbonzini@redhat.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

Extra _GCC. But does no harm.

> +
> +/* 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

Looks okay to me.

I assume __typeof__() is a GCCism, indicated by ..._GCC_H, to ensure
type is actually a valid type?

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 v2 1/3] qemu-tls.h: Add abstraction layer for TLS variables

[Paolo Bonzini]

On 10/27/2011 05:04 PM, Andreas Färber wrote:
> [Paolo's SoB missing]

There's really 1-2 lines of my code left, but anyway

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

Paolo

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

[Peter Maydell]

On 27 October 2011 16:04, Andreas Färber <afaerber@suse.de> wrote:
>> + * Copyright (c) 2011 Red Hat, Inc, Linaro Limited
>
> The concatenation looks kind of funny. ;)

Yeah, I know :-) Hope nobody in five years time goes looking for
those Inc guys to try to track them down for a licensing change...

>> +#ifndef QEMU_TLS_GCC_H
>> +#define QEMU_TLS_GCC_H
>
> Extra _GCC. But does no harm.

That's because I cut-and-pasted it from Paolo's qemu-tls-gcc.h...

>> +#define DEFINE_TLS(type, x)  __thread __typeof__(type) tls__##x

> I assume __typeof__() is a GCCism, indicated by ..._GCC_H, to ensure
> type is actually a valid type?

Paolo?

-- PMM

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

[Paolo Bonzini]

On 10/27/2011 05:15 PM, Peter Maydell wrote:
>
>>> >>  +#define DEFINE_TLS(type, x)  __thread __typeof__(type) tls__##x
>> >  I assume __typeof__() is a GCCism, indicated by ..._GCC_H, to ensure
>> >  type is actually a valid type?
> Paolo?

No, _GCC_H has nothing to do with GCCisms.  __typeof__ is needed so that 
"type" can be "int[5]":

     __thread __typeof__ (int[5]) foo; /* works */
     __thread int[5] foo; /* fails */

(even without __thread of course).

In fact, it has the opposite side effect: type can be any expression, 
but the assumption is that DEFINE_TLS(123, x) will be caught by review.

Paolo

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

[Andreas Färber]

Am 27.10.2011 17:04, schrieb Andreas Färber:
> Am 27.10.2011 13:37, schrieb 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.
>>
> 
> [Paolo's SoB missing]
> 
>> 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..d96a159
>> --- /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
> 
> The concatenation looks kind of funny. ;)
> 
>> + *
>> + * Authors:
>> + *  Paolo Bonzini <pbonzini@redhat.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
> 
> Extra _GCC. But does no harm.
> 
>> +
>> +/* 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
> 
> Looks okay to me.

Reviewed-by: Andreas Färber <afaerber@suse.de>

-- 
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 v2 1/3] qemu-tls.h: Add abstraction layer for TLS variables

[Markus Armbruster]

Andreas Färber <afaerber@suse.de> writes:

> Am 27.10.2011 13:37, schrieb 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.
>> 
>
> [Paolo's SoB missing]
>
>> 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..d96a159
>> --- /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
>
> The concatenation looks kind of funny. ;)

I'd split into

 * Copyright (c) 2011 Red Hat, Inc
 * Copyright (c) 2011 Linaro Limited

>> + *
>> + * Authors:
>> + *  Paolo Bonzini <pbonzini@redhat.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
>
> Extra _GCC. But does no harm.

Unless you file "unusual" under harm, which I happen to do :)

>> +
>> +/* 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

Any particular reason for pasting tls__ onto the identifier?

>> +
>> +#endif
[...]

I wish we'll be able to ditch these cumbersome, ugly macros in favor of
C1X threading support.  It'll probably remain a wish forever, given the
lengths we go to support systems that stubbornly refuse to update their
C programming environment for the 21st century.

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

[Peter Maydell]

On 28 October 2011 08:27, Markus Armbruster <armbru@redhat.com> wrote:
> Andreas Färber <afaerber@suse.de> writes:
>
>> Am 27.10.2011 13:37, schrieb Peter Maydell:
>>> + * Copyright (c) 2011 Red Hat, Inc, Linaro Limited
>>
>> The concatenation looks kind of funny. ;)
>
> I'd split into
>
>  * Copyright (c) 2011 Red Hat, Inc
>  * Copyright (c) 2011 Linaro Limited

>>> +#ifndef QEMU_TLS_GCC_H
>>> +#define QEMU_TLS_GCC_H
>>
>> Extra _GCC. But does no harm.
>
> Unless you file "unusual" under harm, which I happen to do :)

OK, I'll fix these nits and resend later this morning.

>>> +#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
>
> Any particular reason for pasting tls__ onto the identifier?

It means we catch accidentally using the identifier directly
rather than via get_tls() at compile time. (That doesn't matter
for the __thread case, obviously, but does for other implementations.)
(also it means we get out of the way of the cpu_single_env macro
we define in patch 3.)

-- PMM

[Qemu-devel] [PATCH v2 2/3] darwin-user/main.c: Drop unused cpu_single_env definition

[Peter Maydell]

From: Paolo Bonzini <pbonzini@redhat.com>

Drop the cpu_single_env definition as it is unused.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 darwin-user/main.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

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

Re: [Qemu-devel] [PATCH v2 2/3] darwin-user/main.c: Drop unused cpu_single_env definition

[Andreas Färber]

Am 27.10.2011 13:37, schrieb Peter Maydell:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Drop the cpu_single_env definition as it is unused.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Acked-by: Andreas Färber <afaerber@suse.de>

> ---
>  darwin-user/main.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> 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;


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

[Qemu-devel] [PATCH v2 3/3] 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>
[Peter Maydell: rename tls_cpu_single_env to cpu_single_env]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 cpu-all.h |    4 +++-
 exec.c    |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 42a5fa0..5f47ab8 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 *,cpu_single_env);
+#define cpu_single_env get_tls(cpu_single_env)
 
 /* Flags for use in ENV->INTERRUPT_PENDING.
 
diff --git a/exec.c b/exec.c
index 9dc4edb..18e26cb 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 *,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 v2 3/3] Make cpu_single_env thread-local

[Andreas Färber]

Am 27.10.2011 13:37, 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>
> [Peter Maydell: rename tls_cpu_single_env to cpu_single_env]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  cpu-all.h |    4 +++-
>  exec.c    |    2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/cpu-all.h b/cpu-all.h
> index 42a5fa0..5f47ab8 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:
[8<]

Is there a reason not to include this from qemu-common.h? To avoid
accidental use in unsuited cases? If so, add an explicit comment to
either qemu-common.h or Jan's comment in qemu-tls.h to avoid someone
doing such a cleanup/simplification patch?

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 v2 3/3] Make cpu_single_env thread-local

[Paolo Bonzini]

On 10/27/2011 05:10 PM, Andreas Färber wrote:
> Am 27.10.2011 13:37, 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>
>> [Peter Maydell: rename tls_cpu_single_env to cpu_single_env]
>> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
>> ---
>>   cpu-all.h |    4 +++-
>>   exec.c    |    2 +-
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/cpu-all.h b/cpu-all.h
>> index 42a5fa0..5f47ab8 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:
> [8<]
>
> Is there a reason not to include this from qemu-common.h? To avoid
> accidental use in unsuited cases? If so, add an explicit comment to
> either qemu-common.h or Jan's comment in qemu-tls.h to avoid someone
> doing such a cleanup/simplification patch?

I think we're evolving towards having qemu-common.h as a common place 
for system header inclusions, but not really for QEMU-specific headers.

Paolo

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

[Andreas Färber]

Am 27.10.2011 17:10, schrieb Andreas Färber:
> Am 27.10.2011 13:37, 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>
>> [Peter Maydell: rename tls_cpu_single_env to cpu_single_env]
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Andreas Färber <afaerber@suse.de>

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