Patchwork [v2,1/3] qemu-tls.h: Add abstraction layer for TLS variables

login
register
mail settings
Submitter Peter Maydell
Date Oct. 27, 2011, 11:37 a.m.
Message ID <1319715472-16286-2-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/122121/
State New
Headers show

Comments

Peter Maydell - Oct. 27, 2011, 11:37 a.m.
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
Andreas Färber - Oct. 27, 2011, 3:04 p.m.
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
Paolo Bonzini - Oct. 27, 2011, 3:08 p.m.
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
Peter Maydell - Oct. 27, 2011, 3:15 p.m.
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
Paolo Bonzini - Oct. 27, 2011, 3:18 p.m.
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
Andreas Färber - Oct. 27, 2011, 4:18 p.m.
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>
Markus Armbruster - Oct. 28, 2011, 7:27 a.m.
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.
Peter Maydell - Oct. 28, 2011, 7:45 a.m.
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

Patch

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