From patchwork Tue May 17 20:04:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emilio Cota X-Patchwork-Id: 9116021 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 8F9BA9F1C1 for ; Wed, 18 May 2016 02:20:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D858020254 for ; Wed, 18 May 2016 02:20:10 +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 2A10920253 for ; Wed, 18 May 2016 02:20:10 +0000 (UTC) Received: from localhost ([::1]:42311 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2r5l-0005HW-Al for patchwork-qemu-devel@patchwork.kernel.org; Tue, 17 May 2016 22:20:09 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49021) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2qTB-0005C4-VI for qemu-devel@nongnu.org; Tue, 17 May 2016 21:40:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b2lEB-0007OW-3p for qemu-devel@nongnu.org; Tue, 17 May 2016 16:04:31 -0400 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:33107) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2lE8-0007Mv-Q3 for qemu-devel@nongnu.org; Tue, 17 May 2016 16:04:27 -0400 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id AE89F20C25 for ; Tue, 17 May 2016 16:04:15 -0400 (EDT) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Tue, 17 May 2016 16:04:15 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=braap.org; h=cc :content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=TXKZ6 k3pXtJxldTxsSmesrhTfGw=; b=Z8h7bSbkvMY0rGt9KEkgZVyAaKKWUrm43IQBA n5JcSKVocPcpPpVUru4AyYC4if2Jic5tA3jBMrKFrM/9Mq5Op2ybv3KnzlyzEbzR PQB/EJR5UfN3g5RrX+G1bKn/4bJMUk/RAbTcRhAWJDF+YOog4Fvv2ZBAdDVKQQ+u 9ylGLk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=TXKZ6k3pXtJxldTxsSmesrhTfGw=; b=GaK8e yQC5gW7F5pcqIl/bU3EadDK09pW/bO7n0BNNAnkgeZgo4kvyAYeo5/nry2hLTiHX IjwTlR4/19DTXif/S1V/VA9YZOYhBlwPcJzqnmud2SGR4iX+B1We37nj2laM0ftp vVMx3vV50cyyuOjhN7hg0yHnKpZq2ml9Cl5W7Q= X-Sasl-enc: Qqdm8ldigVq+v+ytcUwnUXA/fwEe43wiDQ10Ed/QYgFg 1463515455 Received: from localhost (flamenco.cs.columbia.edu [128.59.20.216]) by mail.messagingengine.com (Postfix) with ESMTPA id 635296801A6; Tue, 17 May 2016 16:04:15 -0400 (EDT) Date: Tue, 17 May 2016 16:04:15 -0400 From: "Emilio G. Cota" To: Richard Henderson Message-ID: <20160517200415.GD30174@flamenco> References: <1463196873-17737-1-git-send-email-cota@braap.org> <1463196873-17737-8-git-send-email-cota@braap.org> <573B5134.8060104@gmail.com> <66d14198-dab0-c72e-fe17-d022cff3feff@twiddle.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <66d14198-dab0-c72e-fe17-d022cff3feff@twiddle.net> User-Agent: Mutt/1.5.23 (2014-03-12) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.111.4.29 Subject: Re: [Qemu-devel] [PATCH v5 07/18] qemu-thread: add simple test-and-set spinlock 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 Devel , Peter Crosthwaite , QEMU Developers , Paolo Bonzini , Sergey Fedorov , Alex =?iso-8859-1?Q?Benn=E9e?= Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" 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 On Tue, May 17, 2016 at 12:19:27 -0700, Richard Henderson wrote: > On 05/17/2016 10:13 AM, Sergey Fedorov wrote: > >> > +static inline void qemu_spin_lock(QemuSpin *spin) > >> > +{ > >> > + while (atomic_test_and_set_acquire(&spin->value)) { > >>From gcc-4.8 info page, node "__atomic Builtins", description of > > __atomic_test_and_set(): > > > > It should be only used for operands of type 'bool' or 'char'. > > > > Hum. I thought I remembered all operand sizes there, but I've just re-checked > and you're right about bool (and really only bool). > > Perhaps we should just stick with __sync_test_and_set then. I'm thinking here > of e.g. armv6, a reasonable host, which can't operate on 1 byte atomic values. I like this idea, it gets rid of any guesswork (as in my previous email). I've changed the patch to: commit 8f89d36b6203b78df2bf1e3f82871b8aa2ca83b7 Author: Emilio G. Cota Date: Thu Apr 28 10:56:26 2016 -0400 atomics: add atomic_test_and_set_acquire Signed-off-by: Emilio G. Cota --- An alternative would be to add just a single line, right below the barrier() definition or at the end of the file. Adding both lines is IMO a bit clearer, since the newly-added comment only applies under the C11 definitions. Thanks, Emilio diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h index 5bc4d6c..95de7a7 100644 --- a/include/qemu/atomic.h +++ b/include/qemu/atomic.h @@ -113,6 +113,13 @@ } while(0) #endif +/* + * We might we tempted to use __atomic_test_and_set with __ATOMIC_ACQUIRE; + * however, the documentation explicitly says that we should only pass + * a boolean to it, so we use __sync_lock_test_and_set, which doesn't + * have this limitation, and is documented to have acquire semantics. + */ +#define atomic_test_and_set_acquire(ptr) __sync_lock_test_and_set(ptr, true) /* All the remaining operations are fully sequentially consistent */ @@ -327,6 +334,8 @@ #endif #endif +#define atomic_test_and_set_acquire(ptr) __sync_lock_test_and_set(ptr, true) + /* Provide shorter names for GCC atomic builtins. */ #define atomic_fetch_inc(ptr) __sync_fetch_and_add(ptr, 1) #define atomic_fetch_dec(ptr) __sync_fetch_and_add(ptr, -1)