From patchwork Fri Sep 30 21:31:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alex_Benn=C3=A9e?= X-Patchwork-Id: 77187 Delivered-To: patch@linaro.org Received: by 10.140.106.72 with SMTP id d66csp516334qgf; Fri, 30 Sep 2016 14:42:44 -0700 (PDT) X-Received: by 10.237.32.205 with SMTP id 71mr9500837qtb.22.1475271764421; Fri, 30 Sep 2016 14:42:44 -0700 (PDT) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id q143si13461255qke.122.2016.09.30.14.42.44 for (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 30 Sep 2016 14:42:44 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; dkim=fail header.i=@linaro.org; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom=qemu-devel-bounces+patch=linaro.org@nongnu.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1]:47081 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bq5Zr-0007tw-Vr for patch@linaro.org; Fri, 30 Sep 2016 17:42:44 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49733) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bq5Oy-0006U6-L7 for qemu-devel@nongnu.org; Fri, 30 Sep 2016 17:31:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bq5Ox-0006ZU-B7 for qemu-devel@nongnu.org; Fri, 30 Sep 2016 17:31:28 -0400 Received: from mail-wm0-x232.google.com ([2a00:1450:400c:c09::232]:37310) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bq5Ox-0006ZE-4X for qemu-devel@nongnu.org; Fri, 30 Sep 2016 17:31:27 -0400 Received: by mail-wm0-x232.google.com with SMTP id b80so66258199wme.0 for ; Fri, 30 Sep 2016 14:31:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=x8Wn9wYtFrz+16aqekWhf850iNjeLIB7uNShWmp4gEg=; b=ZCHI9W253+FvZbImrRced0CY6V5OXBjU4CfhLU5HDiVez+1bW8/1mCVKBdQWUF+enW TM/vz4VRGSTnOByNAn/OGyVkfibQIMSIF83tiZ9Jg1ejr3hqvwVu4aoAOWFFLF1Hv2L8 89VbXUP0M92iSPZ/lmKP23IjeLDkmGuTVH57E= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=x8Wn9wYtFrz+16aqekWhf850iNjeLIB7uNShWmp4gEg=; b=UHxjXsPYhfCTM0+X39caEr7RMfau8l5QySp61FWbKVtMZyxPc1miAB+BwctPnLO5n5 WqLcGCKJhe3MC1tGt+bsll6xG/mTAqj8tNjQwc+F3T0WHib5UxwbjWbWEydiUOYTlCo+ VivWFN1e0mcxLI5ikntMCW0DTuXr4UQO3seAaOb8yZkvNKOJPR0ClZr21CZ3ZblDOaY7 P6YiaQQ5YXoOU2BMX6v0fUn3cqC4Rp82NobedBGULoNxqNH6x3/2pLoBhvLWzFW9mYkZ ZnAbivJUapcrpujdB3dVimelutj/oxtUGPe4IDiYevTG2TnJYgwAnJ0rMUA3NXq3K8SC 7Jug== X-Gm-Message-State: AA6/9RkSbhyrAJTfaIK7rmyKXnJq9EiIJpqw+aBm50jYB9M72IjObS+i8e786q3cuFtJrc3E X-Received: by 10.28.105.18 with SMTP id e18mr5507659wmc.14.1475271086393; Fri, 30 Sep 2016 14:31:26 -0700 (PDT) Received: from zen.linaro.local ([81.128.185.34]) by smtp.gmail.com with ESMTPSA id pm1sm21536961wjb.40.2016.09.30.14.31.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 30 Sep 2016 14:31:23 -0700 (PDT) Received: from zen.linaroharston (localhost [127.0.0.1]) by zen.linaro.local (Postfix) with ESMTP id 1D79F3E0483; Fri, 30 Sep 2016 22:31:18 +0100 (BST) From: =?UTF-8?q?Alex=20Benn=C3=A9e?= To: qemu-devel@nongnu.org, pbonzini@redhat.com Date: Fri, 30 Sep 2016 22:31:00 +0100 Message-Id: <20160930213106.20186-10-alex.bennee@linaro.org> X-Mailer: git-send-email 2.9.3 In-Reply-To: <20160930213106.20186-1-alex.bennee@linaro.org> References: <20160930213106.20186-1-alex.bennee@linaro.org> MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::232 Subject: [Qemu-devel] [PATCH v3 09/15] util/qht: atomically set b->hashes 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: mttcg@listserver.greensocs.com, peter.maydell@linaro.org, claudio.fontana@huawei.com, nikunj@linux.vnet.ibm.com, jan.kiszka@siemens.com, mark.burton@greensocs.com, a.rigo@virtualopensystems.com, cota@braap.org, serge.fdrv@gmail.com, bobby.prani@gmail.com, rth@twiddle.net, =?UTF-8?q?Alex=20Benn=C3=A9e?= , fred.konrad@greensocs.com Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" ThreadSanitizer detects a possible race between reading/writing the hashes. The ordering semantics are already documented for QHT however for true C11 compliance we should use relaxed atomic primitives for accesses that are done across threads. On x86 this slightly changes to the code to not do a load/compare in a single instruction leading to a slight performance degradation. Running 'taskset -c 0 tests/qht-bench -n 1 -d 10' (i.e. all lookups) 10 times, we get: before the patch: $ ./mean.pl 34.04 34.24 34.38 34.25 34.18 34.51 34.46 34.44 34.29 34.08 34.287 +- 0.160072900059109 after: $ ./mean.pl 33.94 34.00 33.52 33.46 33.55 33.71 34.27 34.06 34.28 34.58 33.937 +- 0.374731014640279 Signed-off-by: Alex Bennée Reviewed-by: Emilio G. Cota --- util/qht.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- 2.9.3 diff --git a/util/qht.c b/util/qht.c index 16a8d79..571639d 100644 --- a/util/qht.c +++ b/util/qht.c @@ -379,7 +379,7 @@ static void qht_bucket_reset__locked(struct qht_bucket *head) if (b->pointers[i] == NULL) { goto done; } - b->hashes[i] = 0; + atomic_set(&b->hashes[i], 0); atomic_set(&b->pointers[i], NULL); } b = b->next; @@ -444,7 +444,7 @@ void *qht_do_lookup(struct qht_bucket *head, qht_lookup_func_t func, do { for (i = 0; i < QHT_BUCKET_ENTRIES; i++) { - if (b->hashes[i] == hash) { + if (atomic_read(&b->hashes[i]) == hash) { /* The pointer is dereferenced before seqlock_read_retry, * so (unlike qht_insert__locked) we need to use * atomic_rcu_read here. @@ -538,8 +538,8 @@ static bool qht_insert__locked(struct qht *ht, struct qht_map *map, if (new) { atomic_rcu_set(&prev->next, b); } - b->hashes[i] = hash; /* smp_wmb() implicit in seqlock_write_begin. */ + atomic_set(&b->hashes[i], hash); atomic_set(&b->pointers[i], p); seqlock_write_end(&head->sequence); return true; @@ -607,10 +607,10 @@ qht_entry_move(struct qht_bucket *to, int i, struct qht_bucket *from, int j) qht_debug_assert(to->pointers[i]); qht_debug_assert(from->pointers[j]); - to->hashes[i] = from->hashes[j]; + atomic_set(&to->hashes[i], from->hashes[j]); atomic_set(&to->pointers[i], from->pointers[j]); - from->hashes[j] = 0; + atomic_set(&from->hashes[j], 0); atomic_set(&from->pointers[j], NULL); }