From patchwork Tue Feb 25 01:52:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yanqin Wei X-Patchwork-Id: 1243790 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=arm.com Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48RMPh31tHz9sPk for ; Tue, 25 Feb 2020 12:52:24 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id EFE658788D; Tue, 25 Feb 2020 01:52:21 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tpcJK47rfqoL; Tue, 25 Feb 2020 01:52:21 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 0D634875A7; Tue, 25 Feb 2020 01:52:21 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E60CDC07FF; Tue, 25 Feb 2020 01:52:20 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 77B30C0177 for ; Tue, 25 Feb 2020 01:52:19 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 6680F875A7 for ; Tue, 25 Feb 2020 01:52:19 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2-G8nVizhg4Q for ; Tue, 25 Feb 2020 01:52:18 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by hemlock.osuosl.org (Postfix) with ESMTP id 55FE5864E9 for ; Tue, 25 Feb 2020 01:52:18 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id EE0B530E; Mon, 24 Feb 2020 17:52:17 -0800 (PST) Received: from net-arm-thunderx2-03.shanghai.arm.com (net-arm-thunderx2-03.shanghai.arm.com [10.169.41.185]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id F28FA3F6CF; Mon, 24 Feb 2020 17:52:15 -0800 (PST) From: Yanqin Wei To: dev@openvswitch.org Date: Tue, 25 Feb 2020 09:52:06 +0800 Message-Id: <20200225015206.18986-1-Yanqin.Wei@arm.com> X-Mailer: git-send-email 2.17.1 Cc: Lijian.Zhang@arm.com, Gavin.Hu@arm.com, nd@arm.com Subject: [ovs-dev] [PATCH v1] lib: use acquire-release semantics for pvector size X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Read/write concurrency of pvector library is implemented by a temp vector and RCU protection. Considering performance reason, insertion does not follow this scheme. In insertion function, a thread fence ensures size incrementation is done after new entry is stored. But there is no barrier in the iteration fuction(pvector_cursor_init). Entry point access may be reorderd before loading vector size, so the invalid entry point may be loaded when vector iteration. This patch fixes it by acquire-release pair. It can guarantee new size is observed by reader after new entry stored by writer. And this is implemented by one-way barrier instead of two-way memory fence. Reviewed-by: Gavin Hu Reviewed-by: Lijian Zhang Signed-off-by: Yanqin Wei --- lib/pvector.c | 14 +++++++------- lib/pvector.h | 12 +++++++----- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/pvector.c b/lib/pvector.c index aaeee9214..12c599c97 100644 --- a/lib/pvector.c +++ b/lib/pvector.c @@ -33,7 +33,7 @@ pvector_impl_alloc(size_t size) struct pvector_impl *impl; impl = xmalloc(sizeof *impl + size * sizeof impl->vector[0]); - impl->size = 0; + atomic_init(&impl->size, 0); impl->allocated = size; return impl; @@ -117,18 +117,18 @@ pvector_insert(struct pvector *pvec, void *ptr, int priority) { struct pvector_impl *temp = pvec->temp; struct pvector_impl *old = pvector_impl_get(pvec); + size_t size = old->size; ovs_assert(ptr != NULL); /* Check if can add to the end without reallocation. */ - if (!temp && old->allocated > old->size && - (!old->size || priority <= old->vector[old->size - 1].priority)) { - old->vector[old->size].ptr = ptr; - old->vector[old->size].priority = priority; + if (!temp && old->allocated > size && + (!size || priority <= old->vector[size - 1].priority)) { + old->vector[size].ptr = ptr; + old->vector[size].priority = priority; /* Size increment must not be visible to the readers before the new * entry is stored. */ - atomic_thread_fence(memory_order_release); - ++old->size; + atomic_store_explicit(&old->size, size + 1, memory_order_release); } else { if (!temp) { temp = pvector_impl_dup(old); diff --git a/lib/pvector.h b/lib/pvector.h index b990ed9d5..430bdf746 100644 --- a/lib/pvector.h +++ b/lib/pvector.h @@ -69,8 +69,8 @@ struct pvector_entry { }; struct pvector_impl { - size_t size; /* Number of entries in the vector. */ - size_t allocated; /* Number of allocated entries. */ + ATOMIC(size_t) size; /* Number of entries in the vector. */ + size_t allocated; /* Number of allocated entries. */ struct pvector_entry vector[]; }; @@ -172,7 +172,7 @@ static inline void pvector_cursor_lookahead(const struct pvector_cursor *, #define PVECTOR_CURSOR_FOR_EACH_CONTINUE(PTR, CURSOR) \ for (; ((PTR) = pvector_cursor_next(CURSOR, INT_MIN, 0, 0)) != NULL; ) - + /* Inline implementations. */ static inline struct pvector_cursor @@ -181,12 +181,14 @@ pvector_cursor_init(const struct pvector *pvec, { const struct pvector_impl *impl; struct pvector_cursor cursor; + size_t size; impl = ovsrcu_get(struct pvector_impl *, &pvec->impl); - ovs_prefetch_range(impl->vector, impl->size * sizeof impl->vector[0]); + atomic_read_explicit(&impl->size, &size, memory_order_acquire); + ovs_prefetch_range(impl->vector, size * sizeof impl->vector[0]); - cursor.size = impl->size; + cursor.size = size; cursor.vector = impl->vector; cursor.entry_idx = -1;