qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Hajnoczi
Subject: Re: [RFC 1/2] tls: add macros for coroutine-safe TLS variables
Date: Tue, 26 Oct 2021 17:27:29 +0100

On Tue, Oct 26, 2021 at 08:32:11AM -0700, Richard Henderson wrote:
> 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.

You are right, thanks for pointing this out.

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

Okay, thanks!

Stefan

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]