From patchwork Mon Feb 8 16:14:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 8251801 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id AAE6C9F1C1 for ; Mon, 8 Feb 2016 16:17:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9104A203B7 for ; Mon, 8 Feb 2016 16:17:56 +0000 (UTC) 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.kernel.org (Postfix) with ESMTPS id 6CE1C20148 for ; Mon, 8 Feb 2016 16:17:55 +0000 (UTC) Received: from localhost ([::1]:46209 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSoVe-00016Z-OI for patchwork-qemu-devel@patchwork.kernel.org; Mon, 08 Feb 2016 11:17:54 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57213) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSoTH-0006Bq-Uh for qemu-devel@nongnu.org; Mon, 08 Feb 2016 11:15:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aSoTG-0002Ql-82 for qemu-devel@nongnu.org; Mon, 08 Feb 2016 11:15:27 -0500 Received: from mail-wm0-x243.google.com ([2a00:1450:400c:c09::243]:35434) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSoTF-0002Qb-U9 for qemu-devel@nongnu.org; Mon, 08 Feb 2016 11:15:26 -0500 Received: by mail-wm0-x243.google.com with SMTP id g62so16196275wme.2 for ; Mon, 08 Feb 2016 08:15:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=tyk95Irdg4FpJ1X1Sj5IU3/P56oV9ifN01Gh9iUWeIE=; b=1EgwzkuGBivdz9NRXXldUvCLb7KCCM8Fm5pUjX58rbQzQZKgXnEm5fP/PGDyx6k9Wf BDUqfVhPs35QZOYaoCwWXmb9mPcBY5b3b746uCBkj77dNPIPeUcEvK6PSixwcEkbzV3u h/agp7/hLD/o5VQFKN7H1pEr+GJWVzuYOax9ocoD+vVYXXpnebC2Dg1Hd9tzZdiGc2QC icCt8gbFrGECr8dIb7uFUbkWlwvGL/idIi1A97jfh2gNCHXjvmVAz8dF1s2aQMmw8YCB gc9d0kGgB/UrR2ocbuN5UuwG90lksteqSEs+kcOxIQlT88PVxgKRl8LLL900vyS/3mxG H+Og== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=tyk95Irdg4FpJ1X1Sj5IU3/P56oV9ifN01Gh9iUWeIE=; b=b1SIynHAt6TgCvUMKXJnO4Qq4B4xr9TtesMk2ZHOLjvJVlMV+POq7s0exxgLwKEXCr vNJf4AKxhQ9jAZ3u7NUTtoMo+HI6JsYIvLmKS2671qBP0/1JrgrOPq10Dft3k/vrsxpU OxhDKEA5BEZtO787N+aej+oEy3/tHYc+5e17ShpVtsxf8BlqoUp2WiGceBi4ATdz05jR WcN/NPx1hMdo1S6a+Ah0Ysgfg+GgC94k2jB7lpncJzshDEC96RxGZVxEj4upC4QFxy+x Kr5N+BLWG2eBHzD9FncGrjmJ0hL8uBXKK8r/f3w4gxUUSpvoMAtHQ7Bc+zeLO0NK+uiz gnig== X-Gm-Message-State: AG10YOR3WyVi9Al88r/7yNo3H3Cizk2/5tzISX+dAI1FmG+0LAxORBwcMnFOjluPK1LpNQ== X-Received: by 10.28.63.200 with SMTP id m191mr30318506wma.21.1454948125406; Mon, 08 Feb 2016 08:15:25 -0800 (PST) Received: from donizetti.lan (94-39-141-130.adsl-ull.clienti.tiscali.it. [94.39.141.130]) by smtp.gmail.com with ESMTPSA id v2sm13305429wmd.24.2016.02.08.08.15.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Feb 2016 08:15:24 -0800 (PST) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Mon, 8 Feb 2016 17:14:58 +0100 Message-Id: <1454948107-11844-8-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1454948107-11844-1-git-send-email-pbonzini@redhat.com> References: <1454948107-11844-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::243 Cc: stefanha@redhat.com Subject: [Qemu-devel] [PATCH 07/16] aio: convert from RFifoLock to QemuRecMutex 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+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP It is simpler and a bit faster, and QEMU does not need the contention callbacks (and thus the fairness) anymore. Signed-off-by: Paolo Bonzini --- async.c | 8 ++--- include/block/aio.h | 3 +- include/qemu/rfifolock.h | 54 ---------------------------- tests/.gitignore | 1 - tests/Makefile | 2 -- tests/test-rfifolock.c | 91 ------------------------------------------------ util/Makefile.objs | 1 - util/rfifolock.c | 78 ----------------------------------------- 8 files changed, 5 insertions(+), 233 deletions(-) delete mode 100644 include/qemu/rfifolock.h delete mode 100644 tests/test-rfifolock.c delete mode 100644 util/rfifolock.c diff --git a/async.c b/async.c index e092b1c..edb2846 100644 --- a/async.c +++ b/async.c @@ -254,7 +254,7 @@ aio_ctx_finalize(GSource *source) aio_set_event_notifier(ctx, &ctx->notifier, false, NULL); event_notifier_cleanup(&ctx->notifier); - rfifolock_destroy(&ctx->lock); + qemu_rec_mutex_destroy(&ctx->lock); qemu_mutex_destroy(&ctx->bh_lock); timerlistgroup_deinit(&ctx->tlg); } @@ -552,7 +552,7 @@ AioContext *aio_context_new(Error **errp) event_notifier_dummy_cb); ctx->thread_pool = NULL; qemu_mutex_init(&ctx->bh_lock); - rfifolock_init(&ctx->lock, NULL, NULL); + qemu_rec_mutex_init(&ctx->lock); timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx); qemu_event_init(&ctx->sync_io_event, true); @@ -578,7 +578,7 @@ void aio_context_acquire(AioContext *ctx) if (ctx == qemu_get_aio_context()) { assert(qemu_mutex_iothread_locked()); } else { - rfifolock_lock(&ctx->lock); + qemu_rec_mutex_lock(&ctx->lock); } } @@ -587,6 +587,6 @@ void aio_context_release(AioContext *ctx) if (ctx == qemu_get_aio_context()) { assert(qemu_mutex_iothread_locked()); } else { - rfifolock_unlock(&ctx->lock); + qemu_rec_mutex_unlock(&ctx->lock); } } diff --git a/include/block/aio.h b/include/block/aio.h index 7223daf..3f055d2 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -19,7 +19,6 @@ #include "qemu/queue.h" #include "qemu/event_notifier.h" #include "qemu/thread.h" -#include "qemu/rfifolock.h" #include "qemu/timer.h" typedef struct BlockAIOCB BlockAIOCB; @@ -52,7 +51,7 @@ struct AioContext { GSource source; /* Protects all fields from multi-threaded access */ - RFifoLock lock; + QemuRecMutex lock; /* The list of registered AIO handlers */ QLIST_HEAD(, AioHandler) aio_handlers; diff --git a/include/qemu/rfifolock.h b/include/qemu/rfifolock.h deleted file mode 100644 index b23ab53..0000000 --- a/include/qemu/rfifolock.h +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Recursive FIFO lock - * - * Copyright Red Hat, Inc. 2013 - * - * Authors: - * Stefan Hajnoczi - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - * - */ - -#ifndef QEMU_RFIFOLOCK_H -#define QEMU_RFIFOLOCK_H - -#include "qemu/thread.h" - -/* Recursive FIFO lock - * - * This lock provides more features than a plain mutex: - * - * 1. Fairness - enforces FIFO order. - * 2. Nesting - can be taken recursively. - * 3. Contention callback - optional, called when thread must wait. - * - * The recursive FIFO lock is heavyweight so prefer other synchronization - * primitives if you do not need its features. - */ -typedef struct { - QemuMutex lock; /* protects all fields */ - - /* FIFO order */ - unsigned int head; /* active ticket number */ - unsigned int tail; /* waiting ticket number */ - QemuCond cond; /* used to wait for our ticket number */ - - /* Nesting */ - QemuThread owner_thread; /* thread that currently has ownership */ - unsigned int nesting; /* amount of nesting levels */ - - /* Contention callback */ - void (*cb)(void *); /* called when thread must wait, with ->lock - * held so it may not recursively lock/unlock - */ - void *cb_opaque; -} RFifoLock; - -void rfifolock_init(RFifoLock *r, void (*cb)(void *), void *opaque); -void rfifolock_destroy(RFifoLock *r); -void rfifolock_lock(RFifoLock *r); -void rfifolock_unlock(RFifoLock *r); - -#endif /* QEMU_RFIFOLOCK_H */ diff --git a/tests/.gitignore b/tests/.gitignore index 787c95c..b3da3f1 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -51,7 +51,6 @@ test-qmp-introspect.[ch] test-qmp-marshal.c test-qmp-output-visitor test-rcu-list -test-rfifolock test-string-input-visitor test-string-output-visitor test-thread-pool diff --git a/tests/Makefile b/tests/Makefile index 650e654..9da0fcf 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -39,7 +39,6 @@ check-unit-y += tests/test-visitor-serialization$(EXESUF) check-unit-y += tests/test-iov$(EXESUF) gcov-files-test-iov-y = util/iov.c check-unit-y += tests/test-aio$(EXESUF) -check-unit-$(CONFIG_POSIX) += tests/test-rfifolock$(EXESUF) check-unit-y += tests/test-throttle$(EXESUF) gcov-files-test-aio-$(CONFIG_WIN32) = aio-win32.c gcov-files-test-aio-$(CONFIG_POSIX) = aio-posix.c @@ -403,7 +402,6 @@ tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y) tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y) tests/test-aio$(EXESUF): tests/test-aio.o $(test-block-obj-y) -tests/test-rfifolock$(EXESUF): tests/test-rfifolock.o $(test-util-obj-y) tests/test-throttle$(EXESUF): tests/test-throttle.o $(test-block-obj-y) tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o $(test-block-obj-y) $(test-util-obj-y) tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y) diff --git a/tests/test-rfifolock.c b/tests/test-rfifolock.c deleted file mode 100644 index 0572ebb..0000000 --- a/tests/test-rfifolock.c +++ /dev/null @@ -1,91 +0,0 @@ -/* - * RFifoLock tests - * - * Copyright Red Hat, Inc. 2013 - * - * Authors: - * Stefan Hajnoczi - * - * This work is licensed under the terms of the GNU LGPL, version 2 or later. - * See the COPYING.LIB file in the top-level directory. - */ - -#include -#include "qemu-common.h" -#include "qemu/rfifolock.h" - -static void test_nesting(void) -{ - RFifoLock lock; - - /* Trivial test, ensure the lock is recursive */ - rfifolock_init(&lock, NULL, NULL); - rfifolock_lock(&lock); - rfifolock_lock(&lock); - rfifolock_lock(&lock); - rfifolock_unlock(&lock); - rfifolock_unlock(&lock); - rfifolock_unlock(&lock); - rfifolock_destroy(&lock); -} - -typedef struct { - RFifoLock lock; - int fd[2]; -} CallbackTestData; - -static void rfifolock_cb(void *opaque) -{ - CallbackTestData *data = opaque; - int ret; - char c = 0; - - ret = write(data->fd[1], &c, sizeof(c)); - g_assert(ret == 1); -} - -static void *callback_thread(void *opaque) -{ - CallbackTestData *data = opaque; - - /* The other thread holds the lock so the contention callback will be - * invoked... - */ - rfifolock_lock(&data->lock); - rfifolock_unlock(&data->lock); - return NULL; -} - -static void test_callback(void) -{ - CallbackTestData data; - QemuThread thread; - int ret; - char c; - - rfifolock_init(&data.lock, rfifolock_cb, &data); - ret = qemu_pipe(data.fd); - g_assert(ret == 0); - - /* Hold lock but allow the callback to kick us by writing to the pipe */ - rfifolock_lock(&data.lock); - qemu_thread_create(&thread, "callback_thread", - callback_thread, &data, QEMU_THREAD_JOINABLE); - ret = read(data.fd[0], &c, sizeof(c)); - g_assert(ret == 1); - rfifolock_unlock(&data.lock); - /* If we got here then the callback was invoked, as expected */ - - qemu_thread_join(&thread); - close(data.fd[0]); - close(data.fd[1]); - rfifolock_destroy(&data.lock); -} - -int main(int argc, char **argv) -{ - g_test_init(&argc, &argv, NULL); - g_test_add_func("/nesting", test_nesting); - g_test_add_func("/callback", test_callback); - return g_test_run(); -} diff --git a/util/Makefile.objs b/util/Makefile.objs index a8a777e..c0223c6 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -23,7 +23,6 @@ util-obj-y += crc32c.o util-obj-y += throttle.o util-obj-y += getauxval.o util-obj-y += readline.o -util-obj-y += rfifolock.o util-obj-y += rcu.o util-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o util-obj-y += qemu-coroutine-sleep.o diff --git a/util/rfifolock.c b/util/rfifolock.c deleted file mode 100644 index c22f5fe..0000000 --- a/util/rfifolock.c +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Recursive FIFO lock - * - * Copyright Red Hat, Inc. 2013 - * - * Authors: - * Stefan Hajnoczi - * - * This work is licensed under the terms of the GNU LGPL, version 2 or later. - * See the COPYING.LIB file in the top-level directory. - * - */ - -#include "qemu/osdep.h" -#include "qemu/rfifolock.h" - -void rfifolock_init(RFifoLock *r, void (*cb)(void *), void *opaque) -{ - qemu_mutex_init(&r->lock); - r->head = 0; - r->tail = 0; - qemu_cond_init(&r->cond); - r->nesting = 0; - r->cb = cb; - r->cb_opaque = opaque; -} - -void rfifolock_destroy(RFifoLock *r) -{ - qemu_cond_destroy(&r->cond); - qemu_mutex_destroy(&r->lock); -} - -/* - * Theory of operation: - * - * In order to ensure FIFO ordering, implement a ticketlock. Threads acquiring - * the lock enqueue themselves by incrementing the tail index. When the lock - * is unlocked, the head is incremented and waiting threads are notified. - * - * Recursive locking does not take a ticket since the head is only incremented - * when the outermost recursive caller unlocks. - */ -void rfifolock_lock(RFifoLock *r) -{ - qemu_mutex_lock(&r->lock); - - /* Take a ticket */ - unsigned int ticket = r->tail++; - - if (r->nesting > 0 && qemu_thread_is_self(&r->owner_thread)) { - r->tail--; /* put ticket back, we're nesting */ - } else { - while (ticket != r->head) { - /* Invoke optional contention callback */ - if (r->cb) { - r->cb(r->cb_opaque); - } - qemu_cond_wait(&r->cond, &r->lock); - } - } - - qemu_thread_get_self(&r->owner_thread); - r->nesting++; - qemu_mutex_unlock(&r->lock); -} - -void rfifolock_unlock(RFifoLock *r) -{ - qemu_mutex_lock(&r->lock); - assert(r->nesting > 0); - assert(qemu_thread_is_self(&r->owner_thread)); - if (--r->nesting == 0) { - r->head++; - qemu_cond_broadcast(&r->cond); - } - qemu_mutex_unlock(&r->lock); -}