From patchwork Fri Oct 7 16:57:41 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 77353 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp365105qge; Fri, 7 Oct 2016 10:27:06 -0700 (PDT) X-Received: by 10.200.39.29 with SMTP id g29mr7297448qtg.31.1475861226439; Fri, 07 Oct 2016 10:27:06 -0700 (PDT) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id t69si10579009qki.156.2016.10.07.10.27.06 for (version=TLS1 cipher=AES128-SHA bits=128/128); Fri, 07 Oct 2016 10:27:06 -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; 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 Received: from localhost ([::1]:37478 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsYvJ-0002AT-R1 for patch@linaro.org; Fri, 07 Oct 2016 13:27:05 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34367) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsYTl-00048w-J8 for qemu-devel@nongnu.org; Fri, 07 Oct 2016 12:58:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsYTj-0008BM-C9 for qemu-devel@nongnu.org; Fri, 07 Oct 2016 12:58:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44086) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsYTj-0008Ap-2d for qemu-devel@nongnu.org; Fri, 07 Oct 2016 12:58:35 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7F4DC81129; Fri, 7 Oct 2016 16:58:34 +0000 (UTC) Received: from donizetti.redhat.com (ovpn-112-69.ams2.redhat.com [10.36.112.69]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u97Gw4tj022714; Fri, 7 Oct 2016 12:58:33 -0400 From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Fri, 7 Oct 2016 18:57:41 +0200 Message-Id: <1475859483-32234-18-git-send-email-pbonzini@redhat.com> In-Reply-To: <1475859483-32234-1-git-send-email-pbonzini@redhat.com> References: <1475859483-32234-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Fri, 07 Oct 2016 16:58:34 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 17/39] 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: =?UTF-8?q?Alex=20Benn=C3=A9e?= Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" From: Alex Bennée 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 Message-Id: <20160930213106.20186-10-alex.bennee@linaro.org> Signed-off-by: Paolo Bonzini --- util/qht.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- 2.7.4 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); }