From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58474) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtbcI-0001Ej-OC for qemu-devel@nongnu.org; Mon, 01 Jul 2013 06:45:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UtbcG-0003Jo-Uz for qemu-devel@nongnu.org; Mon, 01 Jul 2013 06:45:54 -0400 Received: from mail-qc0-x232.google.com ([2607:f8b0:400d:c01::232]:49747) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtbcG-0003Jh-Pj for qemu-devel@nongnu.org; Mon, 01 Jul 2013 06:45:52 -0400 Received: by mail-qc0-f178.google.com with SMTP id c11so2738537qcv.37 for ; Mon, 01 Jul 2013 03:45:52 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <51D15DDC.3030705@redhat.com> Date: Mon, 01 Jul 2013 12:45:48 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1372444009-11544-1-git-send-email-pbonzini@redhat.com> <1372444009-11544-6-git-send-email-pbonzini@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 05/30] exec: do not use qemu/tls.h List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org Il 29/06/2013 12:55, Peter Maydell ha scritto: > On 28 June 2013 19:26, Paolo Bonzini wrote: >> The next patch will change qemu/tls.h to support more platforms, but at >> some performance cost. Declare cpu_single_env directly instead of using >> the tls.h abstractions. >> >> Signed-off-by: Paolo Bonzini >> --- >> exec.c | 10 ++++++++-- >> include/exec/cpu-all.h | 14 +++++++++++--- >> include/qemu/tls.h | 52 -------------------------------------------------- >> 3 files changed, 19 insertions(+), 57 deletions(-) >> delete mode 100644 include/qemu/tls.h >> >> diff --git a/exec.c b/exec.c >> index d28403b..a788981 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -70,9 +70,15 @@ static MemoryRegion io_mem_unassigned; >> #endif >> >> CPUArchState *first_cpu; >> + >> /* current CPU in the current thread. It is only valid inside >> - cpu_exec() */ >> -DEFINE_TLS(CPUArchState *,cpu_single_env); >> + * cpu_exec(). See comment in include/exec/cpu-all.h. */ >> +#if defined CONFIG_KVM || (defined CONFIG_USER_ONLY && defined CONFIG_USE_NPTL) >> +__thread CPUArchState *cpu_single_env; >> +#else >> +CPUArchState *cpu_single_env; >> +#endif > > I don't like having the semantics of this variable differ > depending on whether CONFIG_KVM was defined. In particular > this means that the variable is per-thread if you're running > TCG on a QEMU that was configured with KVM support, but > not per-thread if you're running TCG on a QEMU that was > configured without per-thread support. That's just bizarre > and a recipe for confusion and for bugs creeping in in the > less-well-tested config combinations. > > We should just be consistent and always make this be > per-thread. If it's okay to make cpu_single_env accesses more expensive by a factor of 4 on TLS-deficient hosts (at least OpenBSD; do Darwin and NetBSD support thread-local storage?), I'm all for it. I (and I guess Stefan too) do not want to introduce performance regressions in these patches. Making it simpler is something that one would do after having tested at least one of OpenBSD/Darwin/whatever. This patch does not make things worse than before. If anything, it's better because *more* targets have non-TLS semantics: namely non-KVM targets on Linux become non-TLS. Paolo