From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756854AbbCCOe7 (ORCPT ); Tue, 3 Mar 2015 09:34:59 -0500 Received: from foss.arm.com ([217.140.101.70]:50965 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755659AbbCCOe6 (ORCPT ); Tue, 3 Mar 2015 09:34:58 -0500 Date: Tue, 3 Mar 2015 14:34:54 +0000 From: Catalin Marinas To: Pranith Kumar Cc: Will Deacon , Steve Capper , open list , "linux-arm-kernel@lists.infradead.org" Subject: Re: [RFC PATCH] arm64: cmpxchg.h: Bring ldxr and stxr closer Message-ID: <20150303143454.GN28951@e104818-lin.cambridge.arm.com> References: <1425067776-2794-1-git-send-email-bobby.prani@gmail.com> <20150227201526.GH24818@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 27, 2015 at 03:29:38PM -0500, Pranith Kumar wrote: > On Fri, Feb 27, 2015 at 3:17 PM, Pranith Kumar wrote: > > On Fri, Feb 27, 2015 at 3:15 PM, Will Deacon wrote: > >>> @@ -166,11 +166,11 @@ static inline int __cmpxchg_double(volatile void *ptr1, volatile void *ptr2, > >>> VM_BUG_ON((unsigned long *)ptr2 - (unsigned long *)ptr1 != 1); > >>> do { > >>> asm volatile("// __cmpxchg_double8\n" > >>> + " mov %w0, #0\n" > >>> " ldxp %0, %1, %2\n" > >> > >> Seriously, you might want to test this before you mindlessly make changes to > >> low-level synchronisation code. Not only is the change completely unnecessary > >> but it is actively harmful. > >> > > > > Oops, I apologize for this. I should have looked more closely. It is > > wrong to do this in cmpxchg_double(). What about the other cases? > > I tried looking closely on what might be the problem here. I am > waiting on a HiKey arm64 board and I agree I should not send in > changes without running/testing them first. > > Could you please explain (for educational purposes) why you think this > change is harmful? Do you mean the cmpxchg_double() change? Becuase %w0 and %0 is the same physical register. You set it to 0 and immediately override it with ldxp. -- Catalin