From patchwork Tue Apr 26 16:33:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Will Deacon X-Patchwork-Id: 66707 Delivered-To: patch@linaro.org Received: by 10.140.93.198 with SMTP id d64csp1714054qge; Tue, 26 Apr 2016 09:33:55 -0700 (PDT) X-Received: by 10.66.149.194 with SMTP id uc2mr4992909pab.116.1461688435145; Tue, 26 Apr 2016 09:33:55 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id fm7si4871699pad.166.2016.04.26.09.33.54; Tue, 26 Apr 2016 09:33:55 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752582AbcDZQdx (ORCPT + 29 others); Tue, 26 Apr 2016 12:33:53 -0400 Received: from foss.arm.com ([217.140.101.70]:56183 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752158AbcDZQdv (ORCPT ); Tue, 26 Apr 2016 12:33:51 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 891333A; Tue, 26 Apr 2016 09:32:26 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8825B3F42B; Tue, 26 Apr 2016 09:33:44 -0700 (PDT) Date: Tue, 26 Apr 2016 17:33:44 +0100 From: Will Deacon To: Peter Zijlstra Cc: linux-kernel@vger.kernel.org, waiman.long@hpe.com, mingo@redhat.com, paulmck@linux.vnet.ibm.com, boqun.feng@gmail.com, torvalds@linux-foundation.org, dave@stgolabs.net Subject: Re: [RFC][PATCH 3/3] locking,arm64: Introduce cmpwait() Message-ID: <20160426163344.GE1793@arm.com> References: <20160404122250.340636238@infradead.org> <20160404123633.484451002@infradead.org> <20160412165941.GG26124@arm.com> <20160413125243.GA6810@worktop.ger.corp.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160413125243.GA6810@worktop.ger.corp.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Apr 13, 2016 at 02:52:43PM +0200, Peter Zijlstra wrote: > On Tue, Apr 12, 2016 at 05:59:41PM +0100, Will Deacon wrote: > > Thanks for looking at this! > > n/p, had to se what it would look like etc.. :-) > > > > I've misplaced my arm64 compiler, so this is not even compile tested. > > > > Guess what? ;) > > > > > make: *** [init] Error 2 > > make: *** Waiting for unfinished jobs.... > > > > (and lot of similar errors). > > > > Looks like you're just missing an #undef in cmpxchg.h. > > > > FWIW, you can pick up arm64 toolchain binaries from: > > > > https://releases.linaro.org/components/toolchain/binaries/latest-5/ > > Ah, I usually build a whole set from sources; for some reason arm64 > didn't build in the latest run. I'll have to kick it. > > > > +#define __CMPWAIT_GEN(w, sz, name) \ > > > +void __cmpwait_case_##name(volatile void *ptr, unsigned long val) \ > > > +{ \ > > > + unsigned long tmp; \ > > > + \ > > > + asm volatile( \ > > > + " ldxr" #sz "\t%" #w "[tmp], %[v]\n" \ > > > + " eor %" #w "[tmp], %" #w "[tmp], %" #w "[val]\n" \ > > > + " cbnz %" #w "[tmp], 1f\n" \ > > > > Shouldn't this be cbz? (i.e. branch over the wfe if the value is equal > > to what we wanted?). > > Indeed so. > > > > + " wfe\n" \ > > > + "1:" \ > > > + : [tmp] "=&r" (tmp), [val] "=&r" (val), \ > > > > We only read val, so it can be an input operand, no? > > True.. :-) > > > > +#define cmpwait(ptr, val) \ > > > + __cmpwait((ptr), (unsigned long)(val), sizeof(*(ptr))) > > > > We might want to call this cmpwait_relaxed, in case we decide to add > > fenced versions in the future. Or just make it cmpwait_acquire and > > remove the smp_rmb() from smp_cond_load_acquire(). Dunno. > > This is something I'll very much leave up to you. I have no idea on the > tradeoffs involved here. FWIW, here's a fixup patch to get this patch building and running. I also noticed some missing casts for the subword cases. Will --->8 >From 5aa5750d5eeb6e3a42f5547f094dc803f89793bb Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Tue, 26 Apr 2016 17:31:53 +0100 Subject: [PATCH] fixup! locking,arm64: Introduce cmpwait() Signed-off-by: Will Deacon --- arch/arm64/include/asm/cmpxchg.h | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) -- 2.1.4 diff --git a/arch/arm64/include/asm/cmpxchg.h b/arch/arm64/include/asm/cmpxchg.h index cd7bff6ddedc..9b7113a3f0d7 100644 --- a/arch/arm64/include/asm/cmpxchg.h +++ b/arch/arm64/include/asm/cmpxchg.h @@ -225,18 +225,19 @@ __CMPXCHG_GEN(_mb) }) #define __CMPWAIT_GEN(w, sz, name) \ -void __cmpwait_case_##name(volatile void *ptr, unsigned long val) \ +static inline void __cmpwait_case_##name(volatile void *ptr, \ + unsigned long val) \ { \ unsigned long tmp; \ \ asm volatile( \ " ldxr" #sz "\t%" #w "[tmp], %[v]\n" \ " eor %" #w "[tmp], %" #w "[tmp], %" #w "[val]\n" \ - " cbnz %" #w "[tmp], 1f\n" \ + " cbz %" #w "[tmp], 1f\n" \ " wfe\n" \ "1:" \ - : [tmp] "=&r" (tmp), [val] "=&r" (val), \ - [v] "+Q" (*(unsigned long *)ptr)); \ + : [tmp] "=&r" (tmp), [v] "+Q" (*(unsigned long *)ptr) \ + : [val] "r" (val)); \ } __CMPWAIT_GEN(w, b, 1); @@ -244,11 +245,13 @@ __CMPWAIT_GEN(w, h, 2); __CMPWAIT_GEN(w, , 4); __CMPWAIT_GEN( , , 8); +#undef __CMPWAIT_GEN + static inline void __cmpwait(volatile void *ptr, unsigned long val, int size) { switch (size) { - case 1: return __cmpwait_case_1(ptr, val); - case 2: return __cmpwait_case_2(ptr, val); + case 1: return __cmpwait_case_1(ptr, (u8)val); + case 2: return __cmpwait_case_2(ptr, (u16)val); case 4: return __cmpwait_case_4(ptr, val); case 8: return __cmpwait_case_8(ptr, val); default: BUILD_BUG();