From patchwork Mon Nov 5 13:55:37 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 10668315 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1FBB514BD for ; Mon, 5 Nov 2018 13:57:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0EEC42985E for ; Mon, 5 Nov 2018 13:57:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0365929862; Mon, 5 Nov 2018 13:57:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A96E42985E for ; Mon, 5 Nov 2018 13:57:01 +0000 (UTC) Received: from localhost ([::1]:35400 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJfNE-00024P-DZ for patchwork-qemu-devel@patchwork.kernel.org; Mon, 05 Nov 2018 08:57:00 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55097) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJfMA-0001Gv-DD for qemu-devel@nongnu.org; Mon, 05 Nov 2018 08:55:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gJfM9-0006l9-7E for qemu-devel@nongnu.org; Mon, 05 Nov 2018 08:55:54 -0500 Received: from orth.archaic.org.uk ([2001:8b0:1d0::2]:52278) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gJfM8-0006eU-Sn for qemu-devel@nongnu.org; Mon, 05 Nov 2018 08:55:53 -0500 Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89) (envelope-from ) id 1gJfLz-0006Hk-AM; Mon, 05 Nov 2018 13:55:43 +0000 From: Peter Maydell To: qemu-devel@nongnu.org Date: Mon, 5 Nov 2018 13:55:37 +0000 Message-Id: <20181105135538.28025-2-peter.maydell@linaro.org> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181105135538.28025-1-peter.maydell@linaro.org> References: <20181105135538.28025-1-peter.maydell@linaro.org> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:8b0:1d0::2 Subject: [Qemu-devel] [PATCH for-3.1 1/2] include/qemu/thread.h: Document qemu_thread_atexit* API X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Paolo Bonzini , patches@linaro.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Add documentation for the qemu_thread_atexit_add() and qemu_thread_atexit_remove() functions. We include a (previously undocumented) constraint that notifiers may not be called if a thread is exiting because the entire process is exiting. This is fine for our current use because the callers use it only for cleaning up resources which go away on process exit (memory, Win32 fibers), and we will need the flexibility for the new posix implementation. Signed-off-by: Peter Maydell Reviewed-by: Eric Blake --- include/qemu/thread.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/include/qemu/thread.h b/include/qemu/thread.h index b2661b66720..55d83a907cd 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -162,7 +162,29 @@ void qemu_thread_exit(void *retval); void qemu_thread_naming(bool enable); struct Notifier; +/** + * qemu_thread_atexit_add: + * @notifier: Notifier to add + * + * Add the specified notifier to a list which will be run via + * notifier_list_notify() when this thread exits (either by calling + * qemu_thread_exit() or by returning from its start_routine). + * The usual usage is that the caller passes a Notifier which is + * a per-thread variable; it can then use the callback to free + * other per-thread data. + * + * If the thread exits as part of the entire process exiting, + * it is unspecified whether notifiers are called or not. + */ void qemu_thread_atexit_add(struct Notifier *notifier); +/** + * qemu_thread_atexit_remove: + * @notifier: Notifier to remove + * + * Remove the specified notifier from the thread-exit notification + * list. It is not valid to try to remove a notifier which is not + * on the list. + */ void qemu_thread_atexit_remove(struct Notifier *notifier); struct QemuSpin { From patchwork Mon Nov 5 13:55:38 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 10668319 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 25CDE1751 for ; Mon, 5 Nov 2018 13:57:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 15BE529862 for ; Mon, 5 Nov 2018 13:57:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 08FC629865; Mon, 5 Nov 2018 13:57:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8FE4529862 for ; Mon, 5 Nov 2018 13:57:11 +0000 (UTC) Received: from localhost ([::1]:35401 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJfNO-0002Bf-Px for patchwork-qemu-devel@patchwork.kernel.org; Mon, 05 Nov 2018 08:57:10 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55117) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJfMB-0001Ha-MQ for qemu-devel@nongnu.org; Mon, 05 Nov 2018 08:55:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gJfMA-0006nO-LT for qemu-devel@nongnu.org; Mon, 05 Nov 2018 08:55:55 -0500 Received: from orth.archaic.org.uk ([2001:8b0:1d0::2]:52278) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gJfMA-0006eU-CP for qemu-devel@nongnu.org; Mon, 05 Nov 2018 08:55:54 -0500 Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89) (envelope-from ) id 1gJfM0-0006I2-7W; Mon, 05 Nov 2018 13:55:44 +0000 From: Peter Maydell To: qemu-devel@nongnu.org Date: Mon, 5 Nov 2018 13:55:38 +0000 Message-Id: <20181105135538.28025-3-peter.maydell@linaro.org> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181105135538.28025-1-peter.maydell@linaro.org> References: <20181105135538.28025-1-peter.maydell@linaro.org> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:8b0:1d0::2 Subject: [Qemu-devel] [PATCH for-3.1 2/2] util/qemu-thread-posix: Fix qemu_thread_atexit* for OSX X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Paolo Bonzini , patches@linaro.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Our current implementation of qemu_thread_atexit* is broken on OSX. This is because it works by cerating a piece of thread-specific data with pthread_key_create() and using the destructor function for that data to run the notifier function passed to it by the caller of qemu_thread_atexit_add(). The expected use case is that the caller uses a __thread variable as the notifier, and uses the callback to clean up information that it is keeping per-thread in __thread variables. Unfortunately, on OSX this does not work, because on OSX a __thread variable may be destroyed (freed) before the pthread_key_create() destructor runs. (POSIX imposes no ordering constraint here; the OSX implementation happens to implement __thread variables in terms of pthread_key_create((), whereas Linux uses different mechanisms that mean the __thread variables will still be present when the pthread_key_create() destructor is run.) Fix this by switching to a scheme similar to the one qemu-thread-win32 uses for qemu_thread_atexit: keep the thread's notifiers on a __thread variable, and run the notifiers on calls to qemu_thread_exit() and on return from the start routine passed to qemu_thread_start(). We do this with the pthread_cleanup_push() API. We take advantage of the qemu_thread_atexit_add() API permission not to run thread notifiers on process exit to avoid having to special case the main thread. Suggested-by: Paolo Bonzini Signed-off-by: Peter Maydell Reviewed-by: Eric Blake --- qemu-thread-win32.c tries to handle the "main thread" case by using an atexit() handler to run its notifiers. I don't think this will work because the atexit handler may not run on the main thread, in which case notify callback functions which refer to __thread variables will get the wrong ones. --- util/qemu-thread-posix.c | 44 ++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index dfa66ff2fbf..865e476df5b 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -443,42 +443,34 @@ void qemu_event_wait(QemuEvent *ev) } } -static pthread_key_t exit_key; - -union NotifierThreadData { - void *ptr; - NotifierList list; -}; -QEMU_BUILD_BUG_ON(sizeof(union NotifierThreadData) != sizeof(void *)); +static __thread NotifierList thread_exit; +/* + * Note that in this implementation you can register a thread-exit + * notifier for the main thread, but it will never be called. + * This is OK because main thread exit can only happen when the + * entire process is exiting, and the API allows notifiers to not + * be called on process exit. + */ void qemu_thread_atexit_add(Notifier *notifier) { - union NotifierThreadData ntd; - ntd.ptr = pthread_getspecific(exit_key); - notifier_list_add(&ntd.list, notifier); - pthread_setspecific(exit_key, ntd.ptr); + notifier_list_add(&thread_exit, notifier); } void qemu_thread_atexit_remove(Notifier *notifier) { - union NotifierThreadData ntd; - ntd.ptr = pthread_getspecific(exit_key); notifier_remove(notifier); - pthread_setspecific(exit_key, ntd.ptr); } -static void qemu_thread_atexit_run(void *arg) +static void qemu_thread_atexit_notify(void *arg) { - union NotifierThreadData ntd = { .ptr = arg }; - notifier_list_notify(&ntd.list, NULL); + /* + * Called when non-main thread exits (via qemu_thread_exit() + * or by returning from its start routine.) + */ + notifier_list_notify(&thread_exit, NULL); } -static void __attribute__((constructor)) qemu_thread_atexit_init(void) -{ - pthread_key_create(&exit_key, qemu_thread_atexit_run); -} - - typedef struct { void *(*start_routine)(void *); void *arg; @@ -490,6 +482,7 @@ static void *qemu_thread_start(void *args) QemuThreadArgs *qemu_thread_args = args; void *(*start_routine)(void *) = qemu_thread_args->start_routine; void *arg = qemu_thread_args->arg; + void *r; #ifdef CONFIG_PTHREAD_SETNAME_NP /* Attempt to set the threads name; note that this is for debug, so @@ -501,7 +494,10 @@ static void *qemu_thread_start(void *args) #endif g_free(qemu_thread_args->name); g_free(qemu_thread_args); - return start_routine(arg); + pthread_cleanup_push(qemu_thread_atexit_notify, NULL); + r = start_routine(arg); + pthread_cleanup_pop(1); + return r; } void qemu_thread_create(QemuThread *thread, const char *name,