Re: [Qemu-devel] [PATCH] Make cpu_single_env thread local (Linux only for now)

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2921 bytes --]

On 2011-10-04 17:10, Lluís Vilanova wrote:
> Jan Kiszka writes:
> 
>> On 2011-10-03 18:33, Dr. David Alan Gilbert wrote:
>>> Make cpu_single_env thread local (Linux only for now)
>>> * Fixes some user space threading issues (esp those triggered
>>> by bug 823902)
>>>
>>> Against rev d11cf8cc..., tested on ARM user mode, and ARM Vexpress
>>> system mode (with Blue Swirl's fix from yesterday) - only
>>> tested on Linux host.  Lets me run ARM userspace firefox.
>>>
>>> Signed-off-by: Dr. David Alan Gilbert <david.gilbert@linaro.org>
>>>
>>> diff --git a/cpu-all.h b/cpu-all.h
>>> index 42a5fa0..d895ee6 100644
>>> --- a/cpu-all.h
>>> +++ b/cpu-all.h
>>> @@ -334,7 +334,13 @@ 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;
>>> +
>>> +#ifdef __linux__
>>> +/* DAG: Only tested thread local on Linux, feel free to add others */
>>> +extern __thread CPUState *cpu_single_env;
>>> +#else
>>> extern CPUState *cpu_single_env;
>>> +#endif
> 
>> We need this for all platforms in order to skip qemu_global_mutex while
>> manipulating some CPUState. And leaving some platforms with non-TLS will
>> eventually break them when code is added that assumes TLS.
> 
>> However, it's not unlikely that some weird platforms / ancient
>> toolchains still have problems with __thread - even on Linux. We may
>> want to play safe and use pthread_key on POSIX.
> 
> Why not make this kind of annotations available in qemu-commmon.h; or even
> better somewhere less... "generic".

Yes, we will need some encapsulation for this.

> 
> #if defined(CONFIG_SUPPORTS_THREAD_KEYWORD)
> 
> // use __thread
> #define QEMU_DECL_TLS(type, name) \
>         extern __thread type name;
> #define QEMU_DEF_TLS(type, name) \
>         __thread type name;
> #define QEMU_SET_TLS(name, value) \
>         do { name = value; } while (0)
> #define QEMU_GET_TLS(name) \
>         name
> 
> #elif defined(CONFIG_SUPPORTS_PTHREAD_KEY)
> 
> // use pthread_key_t
> #define QEMU_DECL_TLS(type, name) \
>         extern type _type_##name; \
>         extern pthread_key_t name;
> #define QEMU_DEF_TLS(type, name) \
>         pthread_key_t name; \
>         void _init_##name (void) __attribute__((constructor)); \
>         void _init_##name (void) { pthread_key_create(&name, NULL); }
> #define QEMU_SET_TLS(name, value) \
>         do { pthread_setspecific(name, (void*)value); } while (0)
> #define QEMU_GET_TLS(name) \
>         ((typeof(_type_##name))pthread_getspecific(name))
> 
> #else
> #error Go home
> #endif

Looks like a start. But I would avoid macros and go for (static inline)
functions where possible. And initialization should be explicit (so that
you can start using TLS already inside constructors).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]