Re: [RFC 1/2] tls: add macros for coroutine-safe TLS variables

[Richard Henderson]

On 10/26/21 6:30 AM, Stefan Hajnoczi wrote:
> On Mon, Oct 25, 2021 at 10:19:04AM -0700, Richard Henderson wrote:
> > On 10/25/21 7:07 AM, Stefan Hajnoczi wrote:
> > > Compiler optimizations can cache TLS values across coroutine yield
> > > points, resulting in stale values from the previous thread when a
> > > coroutine is re-entered by a new thread.
> > ...
> > >    include/qemu/tls.h | 142 +++++++++++++++++++++++++++++++++++++++++++++
> >
> > Better as qemu/coroutine-tls.h, since it is needed for no other purpose.
> >
> > > +#define QEMU_DEFINE_TLS(type, var) \
> > > +    __thread type qemu_tls_##var; \
> > > +    type get_##var(void) { return qemu_tls_##var; } \
> > > +    void set_##var(type v) { qemu_tls_##var = v; }
> >
> > You might as well make the variable static, since it may only be referenced
> > by these two functions.
>
> Oops, that's a bug. It should be declared extern. QEMU_DEFINE_TLS() is
> meant for use in header files.

No, QEMU_DECLARE_TLS was for use in header files, and it of course does not globally 
declare the tls variable at all.  Only the definition here requires the tls variable.


> Nice. That makes me wonder if it's possible to write a portable version:
>
>    static inline TYPE get_##VAR(void) \
>    { volatile TYPE *p = &co_tls_##VAR; return *p; }
>
> But the trouble is we need &co_tls_##VAR to be "volatile" and I don't
> think there is a way to express that?

No, there's not.

Anyway, with those four hosts we get coverage of almost all users.  I'll note that both 
arm32 and s390x use the constant pool in computing these tls addresses, so they'll need to 
keep using your functional version.  So we'll still have testing of that path as well.


r~