From patchwork Wed Aug 14 15:50:44 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Mike D. Day" X-Patchwork-Id: 267157 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 832B02C0170 for ; Thu, 15 Aug 2013 03:35:23 +1000 (EST) Received: from localhost ([::1]:40957 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9eyf-0000WH-En for incoming@patchwork.ozlabs.org; Wed, 14 Aug 2013 13:35:21 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55754) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9dMU-0007R2-F2 for qemu-devel@nongnu.org; Wed, 14 Aug 2013 11:51:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V9dMN-0007va-U6 for qemu-devel@nongnu.org; Wed, 14 Aug 2013 11:51:50 -0400 Received: from mail-qe0-f51.google.com ([209.85.128.51]:61090) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9dMN-0007vU-Ob for qemu-devel@nongnu.org; Wed, 14 Aug 2013 11:51:43 -0400 Received: by mail-qe0-f51.google.com with SMTP id nd7so5101630qeb.24 for ; Wed, 14 Aug 2013 08:51:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=dKzfWr5miRR2MXk6gsdTJY0g4fm44CzY/HlJP+KBDLY=; b=IGpsES8V9mDeXfH/ZWm5k0RMSSebR/DEMByHGasaL+Hz7OCusb2kMOcN+GUBttfAf8 E3rDslX4UYKznDIdR6TXkS6uXwCGLSmcrWNUjalqSiGqtwQ4Hogq6q2pl2BYn7t5e/KH Gfs5SjDXN64+f8GIEHhMUvoADVACmREVY8K/SxX6u6lEd/iDrMsI9V1LvWXd3mV1kXAe OSaM9bLvAv3OsTOulALnRqrpoqnTM74npLHHD06bVe/4UeUuF+UtKM0PvHFzNCnbqTtd VbtpXQfBTtjgz0XuKzMlA7vXtHJmCRcBCNHj2GtRyhdJXdpTwIGMp5/dJ7lajz+iMhT8 FKJA== X-Gm-Message-State: ALoCoQmJmmxk/g6wHlv5kJg0QI3ejNryxCTgGP1OpK+JuBIK2RYWoIyMf/+ThKEC64PO9mMzpTpt X-Received: by 10.224.24.72 with SMTP id u8mr2673318qab.115.1376495503245; Wed, 14 Aug 2013 08:51:43 -0700 (PDT) Received: from ncultra.org (cpe-076-182-073-192.nc.res.rr.com. [76.182.73.192]) by mx.google.com with ESMTPSA id a8sm51534383qae.11.2013.08.14.08.51.42 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 14 Aug 2013 08:51:42 -0700 (PDT) From: Mike Day To: qemu-devel@nongnu.org Date: Wed, 14 Aug 2013 11:50:44 -0400 Message-Id: <1376495450-5133-9-git-send-email-ncmike@ncultra.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1376495450-5133-1-git-send-email-ncmike@ncultra.org> References: <1376495450-5133-1-git-send-email-ncmike@ncultra.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.128.51 X-Mailman-Approved-At: Wed, 14 Aug 2013 13:31:49 -0400 Cc: Paolo Bonzini Subject: [Qemu-devel] [RFC PATCH 08/14] qemu-thread: report RCU quiescent states X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Paolo Bonzini Most threads will use mutexes and other sleeping synchronization primitives (condition variables, semaphores, events) periodically. For these threads, the synchronization primitives are natural places to report a quiescent state (possibly an extended one). Signed-off-by: Paolo Bonzini Reviewed-by: Mike Day --- docs/rcu.txt | 27 +++++++++++++++++++++++++++ util/qemu-thread-posix.c | 29 +++++++++++++++++++++++++---- util/qemu-thread-win32.c | 16 +++++++++++++++- util/rcu.c | 3 --- 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/docs/rcu.txt b/docs/rcu.txt index 4e7cde3..38fd8f4 100644 --- a/docs/rcu.txt +++ b/docs/rcu.txt @@ -168,6 +168,33 @@ of "quiescent states", i.e. points where no RCU read-side critical section can be active. All threads created with qemu_thread_create participate in the RCU mechanism and need to annotate such points. +Luckily, in most cases no manual annotation is needed, because waiting +on condition variables (qemu_cond_wait), semaphores (qemu_sem_wait, +qemu_sem_timedwait) or events (qemu_event_wait) implicitly marks the thread +as quiescent for the whole duration of the wait. (There is an exception +for semaphore waits with a zero timeout). + +Manual annotation is still needed in the following cases: + +- threads that spend their sleeping time in the kernel, for example + in a call to select(), poll() or WaitForMultipleObjects(). The QEMU + I/O thread is an example of this case. + +- threads that perform a lot of I/O. In QEMU, the workers used for + aio=thread are an example of this case (see aio_worker in block/raw-*). + +- threads that run continuously until they exit. The migration thread + is an example of this case. + +Regarding the second case, note that the workers run in the QEMU thread +pool. The thread pool uses semaphores for synchronization, hence it does +report quiescent states periodically. However, in some cases (e.g. NFS +mounted with the "hard" option) the workers can take an arbitrarily long +amount of time. When this happens, synchronize_rcu() will not exit and +call_rcu() callbacks will be delayed arbitrarily. It is therefore a +good idea to mark I/O system calls as quiescence points in the worker +functions. + Marking quiescent states is done with the following three APIs: void rcu_quiescent_state(void); diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 2371176..dbefff8 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -119,7 +119,9 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) { int err; + rcu_thread_offline(); err = pthread_cond_wait(&cond->cond, &mutex->lock); + rcu_thread_online(); if (err) error_exit(err, __func__); } @@ -211,6 +213,10 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) int rc; struct timespec ts; + if (ms) { + rcu_thread_offline(); + } + #if defined(__APPLE__) || defined(__NetBSD__) rc = 0; compute_abs_deadline(&ts, ms); @@ -228,7 +234,10 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) --sem->count; } pthread_mutex_unlock(&sem->lock); - return (rc == ETIMEDOUT ? -1 : 0); + if (rc == ETIMEDOUT) { + rc == -1; + } + #else if (ms <= 0) { /* This is cheaper than sem_timedwait. */ @@ -236,7 +245,7 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) rc = sem_trywait(&sem->sem); } while (rc == -1 && errno == EINTR); if (rc == -1 && errno == EAGAIN) { - return -1; + goto out; } } else { compute_abs_deadline(&ts, ms); @@ -244,19 +253,26 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) rc = sem_timedwait(&sem->sem, &ts); } while (rc == -1 && errno == EINTR); if (rc == -1 && errno == ETIMEDOUT) { - return -1; + goto out; } } if (rc < 0) { error_exit(errno, __func__); } - return 0; + #endif + +out: + if (ms) { + rcu_thread_online(); + } + return rc; } void qemu_sem_wait(QemuSemaphore *sem) { int rc; + rcu_thread_offline(); #if defined(__APPLE__) || defined(__NetBSD__) pthread_mutex_lock(&sem->lock); @@ -276,6 +292,7 @@ void qemu_sem_wait(QemuSemaphore *sem) error_exit(errno, __func__); } #endif + rcu_thread_online(); } #ifdef __linux__ @@ -384,7 +401,11 @@ void qemu_event_wait(QemuEvent *ev) return; } } + rcu_thread_offline(); futex_wait(ev, EV_BUSY); + rcu_thread_online(); + } else { + rcu_quiescent_state(); } } diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c index 0c4850d..7bc07b3 100644 --- a/util/qemu-thread-win32.c +++ b/util/qemu-thread-win32.c @@ -12,6 +12,7 @@ */ #include "qemu-common.h" #include "qemu/thread.h" +#include "qemu/rcu.h" #include #include #include @@ -170,7 +171,9 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) * leaving mutex unlocked before we wait on semaphore. */ qemu_mutex_unlock(mutex); + rcu_thread_offline(); WaitForSingleObject(cond->sema, INFINITE); + rcu_thread_online(); /* Now waiters must rendez-vous with the signaling thread and * let it continue. For cond_broadcast this has heavy contention @@ -210,7 +213,16 @@ void qemu_sem_post(QemuSemaphore *sem) int qemu_sem_timedwait(QemuSemaphore *sem, int ms) { - int rc = WaitForSingleObject(sem->sema, ms); + int rc; + + if (ms) { + rcu_thread_offline(); + } + rc = WaitForSingleObject(sem->sema, ms); + if (ms) { + rcu_thread_offline(); + } + if (rc == WAIT_OBJECT_0) { return 0; } @@ -250,7 +262,9 @@ void qemu_event_reset(QemuEvent *ev) void qemu_event_wait(QemuEvent *ev) { + rcu_thread_offline(); WaitForSingleObject(ev->event, INFINITE); + rcu_thread_online(); } struct QemuThreadData { diff --git a/util/rcu.c b/util/rcu.c index 27fda86..91d6ae2 100644 --- a/util/rcu.c +++ b/util/rcu.c @@ -240,9 +240,6 @@ static void *call_rcu_thread(void *opaque) { struct rcu_head *node; - /* This thread is just a writer. */ - rcu_thread_offline(); - for (;;) { int tries = 0; int n = atomic_read(&rcu_call_count);