[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 0/4] tls: add macros for coroutine-safe TLS variables
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v5 0/4] tls: add macros for coroutine-safe TLS variables |
Date: |
Thu, 3 Mar 2022 13:07:54 +0100 |
Am 22.02.2022 um 15:01 hat Stefan Hajnoczi geschrieben:
> v5:
> - Added explicit "#include "qemu/coroutine-tls.h" in patch 4 [Philippe]
> - Updated patch 1 commit description and comments to describe the current
> noinline plus asm volatile approach [Peter]
> v4:
> - Dropped '[RFC]'.
> - Dropped inline asm for now. -fPIC versions of the code are missing and I
> hit several issues including a clang LTO bug where thread local variables
> are
> incorrectly discarded because inline asm is not analyzed to find symbol
> dependencies (Serge Guelton is aware).
> - Fixed CI failures.
> v3:
> - Added __attribute__((weak)) to get_ptr_*() [Florian]
> - Replace rdfsbase with mov %%fs:0,%0 [Florian]
>
> This patch series solves the coroutines TLS problem. Coroutines re-entered
> from
> another thread sometimes see stale TLS values. This happens because compilers
> may cache values across yield points, so a value from the previous thread will
> be used when the coroutine is re-entered in another thread.
>
> Serge Guelton developed a portable technique, see the first patch for details.
>
> I have audited all __thread variables in QEMU and converted those that can be
> used from coroutines. Most actually look safe to me.
Hm, what about the ones in the coroutine implementation itself?
static __thread CoroutineUContext leader;
static __thread Coroutine *current;
Both of them are used in qemu_coroutine_self(), which is a
coroutine_fn, and in qemu_in_coroutine(), which may be called from
coroutine context.
And I seem to remember I've seen crashes related to this in one of the
bug reports we got, where the stack trace clearly showed that one of
these functions had returned a wrong result.
I'm applying this series anyway, it doesn't make the patches incorrect.
But it feels incomplete, so we may need a follow-up patch.
Kevin