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
signature.asc
Description: PGP signature