Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752466AbbHFNPw (ORCPT ); Thu, 6 Aug 2015 09:15:52 -0400 Received: from e06smtp13.uk.ibm.com ([195.75.94.109]:44058 "EHLO e06smtp13.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750758AbbHFNPu (ORCPT ); Thu, 6 Aug 2015 09:15:50 -0400 X-Helo: d06dlp01.portsmouth.uk.ibm.com X-MailFrom: dahi@linux.vnet.ibm.com X-RcptTo: linux-kernel@vger.kernel.org Date: Thu, 6 Aug 2015 15:15:45 +0200 From: David Hildenbrand To: Vineet Gupta Cc: "Peter Zijlstra (Intel)" , Thomas Gleixner , Michel Lespinasse , , lkml Subject: Re: [PATCH 1/4] ARC: add barriers to futex code Message-ID: <20150806151545.0f5f3845@thinkpad-w530> In-Reply-To: <1438864523-31340-2-git-send-email-vgupta@synopsys.com> References: <1438864523-31340-1-git-send-email-vgupta@synopsys.com> <1438864523-31340-2-git-send-email-vgupta@synopsys.com> Organization: IBM Deutschland GmbH X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.28; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15080613-0013-0000-0000-000004F08FD6 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2806 Lines: 100 > The atomic ops on futex need to provide the full barrier just like > regular atomics in kernel. > > Also remove pagefault_enable/disable in futex_atomic_cmpxchg_inatomic() > as core code already does that > > Cc: David Hildenbrand > Cc: Peter Zijlstra (Intel) > Cc: Thomas Gleixner > Cc: Michel Lespinasse > Signed-off-by: Vineet Gupta > --- > arch/arc/include/asm/futex.h | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/arch/arc/include/asm/futex.h b/arch/arc/include/asm/futex.h > index 70cfe16b742d..160656d0a15a 100644 > --- a/arch/arc/include/asm/futex.h > +++ b/arch/arc/include/asm/futex.h > @@ -20,6 +20,7 @@ > > #define __futex_atomic_op(insn, ret, oldval, uaddr, oparg)\ > \ > + smp_mb(); \ > __asm__ __volatile__( \ > "1: llock %1, [%2] \n" \ > insn "\n" \ > @@ -40,12 +41,14 @@ > \ > : "=&r" (ret), "=&r" (oldval) \ > : "r" (uaddr), "r" (oparg), "ir" (-EFAULT) \ > - : "cc", "memory") > + : "cc", "memory"); \ > + smp_mb(); \ I think you should drop the ; > > #else /* !CONFIG_ARC_HAS_LLSC */ > > #define __futex_atomic_op(insn, ret, oldval, uaddr, oparg)\ > \ > + smp_mb(); \ > __asm__ __volatile__( \ > "1: ld %1, [%2] \n" \ > insn "\n" \ > @@ -65,7 +68,8 @@ > \ > : "=&r" (ret), "=&r" (oldval) \ > : "r" (uaddr), "r" (oparg), "ir" (-EFAULT) \ > - : "cc", "memory") > + : "cc", "memory"); \ > + smp_mb(); \ dito > > #endif > > @@ -151,7 +155,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr, u32 oldval, > if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int))) > return -EFAULT; > > - pagefault_disable(); > + smp_mb(); > > __asm__ __volatile__( > #ifdef CONFIG_ARC_HAS_LLSC > @@ -178,7 +182,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr, u32 oldval, > : "r"(oldval), "r"(newval), "r"(uaddr), "ir"(-EFAULT) > : "cc", "memory"); > > - pagefault_enable(); > + smp_mb(); > > *uval = val; > return val; Looks like pagefault_() magic is only required for futex_atomic_op_inuser. So this should be fine (and arc seems to be the only arch left that has in in _inatomic). Not sure if you want to change the comment: /* Compare-xchg with pagefaults disabled. * Notes: * -Best-Effort: Exchg happens only if compare succeeds. Maybe something like "Compare-xchg: pagefaults have to be disabled by the caller" Looks sane to me. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/