From patchwork Wed Jan 9 06:24:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 10753451 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 7F9AC17D2 for ; Wed, 9 Jan 2019 06:25:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6E1AC28E16 for ; Wed, 9 Jan 2019 06:25:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 62ACA28E19; Wed, 9 Jan 2019 06:25:21 +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=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from pdx1-mailman02.dreamhost.com (pdx1-mailman02.dreamhost.com [64.90.62.194]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 0BA6128E16 for ; Wed, 9 Jan 2019 06:25:21 +0000 (UTC) Received: from pdx1-mailman02.dreamhost.com (localhost [IPv6:::1]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id 5357021FDD9; Tue, 8 Jan 2019 22:25:20 -0800 (PST) X-Original-To: lustre-devel@lists.lustre.org Delivered-To: lustre-devel-lustre.org@pdx1-mailman02.dreamhost.com Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id 8C95721FDB2 for ; Tue, 8 Jan 2019 22:25:18 -0800 (PST) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id C5CA4AF0B; Wed, 9 Jan 2019 06:25:17 +0000 (UTC) From: NeilBrown To: James Simmons , Oleg Drokin , Andreas Dilger Date: Wed, 09 Jan 2019 17:24:01 +1100 Message-ID: <154701504111.26726.7861020038366907898.stgit@noble> In-Reply-To: <154701488711.26726.17363928508883972338.stgit@noble> References: <154701488711.26726.17363928508883972338.stgit@noble> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Subject: [lustre-devel] [PATCH 03/29] lustre: osc: simplify osc_extent_wait() X-BeenThere: lustre-devel@lists.lustre.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "For discussing Lustre software development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lustre Development List Errors-To: lustre-devel-bounces@lists.lustre.org Sender: "lustre-devel" X-Virus-Scanned: ClamAV using ClamSMTP Taking a spinlock to check the current value of the state is unnecessary. The wake_up() and wait_event() calls have sufficient barriers to ensure that the value will be seen and the wait will abort properly. In most cases, osc_extent_wait() is followed by osc_object_lock() before any shared data is touched - in those cases there is no need for osc_extent_wait() to wait for the spinlock to be released. The one case where osc_object_lock() does not immediately follow is in osc_cache_truncate_start(). The extra locking was introduced in a patch which fixed a problem with truncation, so it is likely that this is the call that was thought to be relevant. In that case, following osc_extent_wait(), an extent that had been detached from the per-object list (oe_link linkage) and proceeds to work on it without any locking. In this case the code is waiting for OES_TRUNC, so any changes that happen after the osc_extent_state_set(ext, OES_TRUNC) and when the lock is dropped, might not be seen by the woken code. The only thing changed is ->oe_trunc_pending, and the woken code doesn't look at that. The only remaining possible need for extra synchronization is if some other value was changed before the wakeup and is needed after the wait. According to memory-barriers.txt, a barrier might be needed to ensure that is visible. Such a barrier is most clearly presented by used smp_store_release() to set the state before wakeup, and smp_load_acquire() to view it after waiting. Also use a simple wake_up() instead of wake_up_all() - the latter is only needed when exclusive waiting is being used. Signed-off-by: NeilBrown --- drivers/staging/lustre/lustre/osc/osc_cache.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c index 1ce9f673f1bf..00056dffceb9 100644 --- a/drivers/staging/lustre/lustre/osc/osc_cache.c +++ b/drivers/staging/lustre/lustre/osc/osc_cache.c @@ -345,8 +345,8 @@ static void osc_extent_state_set(struct osc_extent *ext, int state) /* LASSERT(sanity_check_nolock(ext) == 0); */ /* TODO: validate the state machine */ - ext->oe_state = state; - wake_up_all(&ext->oe_waitq); + smp_store_release(&ext->oe_state, state); + wake_up(&ext->oe_waitq); } static struct osc_extent *osc_extent_alloc(struct osc_object *obj) @@ -948,17 +948,6 @@ int osc_extent_finish(const struct lu_env *env, struct osc_extent *ext, return 0; } -static int extent_wait_cb(struct osc_extent *ext, enum osc_extent_state state) -{ - int ret; - - osc_object_lock(ext->oe_obj); - ret = ext->oe_state == state; - osc_object_unlock(ext->oe_obj); - - return ret; -} - /** * Wait for the extent's state to become @state. */ @@ -989,13 +978,16 @@ static int osc_extent_wait(const struct lu_env *env, struct osc_extent *ext, /* wait for the extent until its state becomes @state */ rc = wait_event_idle_timeout(ext->oe_waitq, - extent_wait_cb(ext, state), 600 * HZ); + smp_load_acquire(&ext->oe_state) == state, + 600 * HZ); if (rc == 0) { OSC_EXTENT_DUMP(D_ERROR, ext, "%s: wait ext to %u timedout, recovery in progress?\n", cli_name(osc_cli(obj)), state); - wait_event_idle(ext->oe_waitq, extent_wait_cb(ext, state)); + wait_event_idle(ext->oe_waitq, + smp_load_acquire(&ext->oe_state) == state); + } if (ext->oe_rc < 0) rc = ext->oe_rc;