From mboxrd@z Thu Jan 1 00:00:00 1970 From: Waiman Long Subject: Re: [RFC PATCH-tip v2 1/6] locking/osq: Make lock/unlock proper acquire/release barrier Date: Wed, 15 Jun 2016 16:04:07 -0400 Message-ID: <5761B4B7.3070102@hpe.com> References: <1465944489-43440-1-git-send-email-Waiman.Long@hpe.com> <1465944489-43440-2-git-send-email-Waiman.Long@hpe.com> <20160615165659.GC2094@linux-80c1.suse> <20160615171250.GO30921@twins.programming.kicks-ass.net> <5761A7C0.6000709@hpe.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5761A7C0.6000709@hpe.com> Sender: linux-kernel-owner@vger.kernel.org To: Peter Zijlstra Cc: Davidlohr Bueso , Ingo Molnar , linux-kernel@vger.kernel.org, x86@kernel.org, linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org, linux-arch@vger.kernel.org, Jason Low , Dave Chinner , Scott J Norton , Douglas Hatch List-Id: linux-arch.vger.kernel.org On 06/15/2016 03:08 PM, Waiman Long wrote: > On 06/15/2016 01:12 PM, Peter Zijlstra wrote: >> On Wed, Jun 15, 2016 at 09:56:59AM -0700, Davidlohr Bueso wrote: >>> On Tue, 14 Jun 2016, Waiman Long wrote: >>>> +++ b/kernel/locking/osq_lock.c >>>> @@ -115,7 +115,7 @@ bool osq_lock(struct optimistic_spin_queue *lock) >>>> * cmpxchg in an attempt to undo our queueing. >>>> */ >>>> >>>> - while (!READ_ONCE(node->locked)) { >>>> + while (!smp_load_acquire(&node->locked)) { >>> Hmm this being a polling path, that barrier can get pretty expensive >>> and >>> last I checked it was unnecessary: >> I think he'll go rely on it later on. >> >> In any case, its fairly simple to cure, just add >> smp_acquire__after_ctrl_dep() at the end. If we bail because >> need_resched() we don't need the acquire I think. > > Yes, I only need the acquire barrier when the locking is successful. > Thanks for the suggestion. I will make the change accordingly. > BTW, when will the smp_acquire__after_ctrl_dep() patch goes into the tip tree? My patch will have a dependency on that when I make the change. Cheers, Longman From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-bn1bon0115.outbound.protection.outlook.com ([157.56.111.115]:5792 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932084AbcFOUEW (ORCPT ); Wed, 15 Jun 2016 16:04:22 -0400 Message-ID: <5761B4B7.3070102@hpe.com> Date: Wed, 15 Jun 2016 16:04:07 -0400 From: Waiman Long MIME-Version: 1.0 Subject: Re: [RFC PATCH-tip v2 1/6] locking/osq: Make lock/unlock proper acquire/release barrier References: <1465944489-43440-1-git-send-email-Waiman.Long@hpe.com> <1465944489-43440-2-git-send-email-Waiman.Long@hpe.com> <20160615165659.GC2094@linux-80c1.suse> <20160615171250.GO30921@twins.programming.kicks-ass.net> <5761A7C0.6000709@hpe.com> In-Reply-To: <5761A7C0.6000709@hpe.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Peter Zijlstra Cc: Davidlohr Bueso , Ingo Molnar , linux-kernel@vger.kernel.org, x86@kernel.org, linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org, linux-arch@vger.kernel.org, Jason Low , Dave Chinner , Scott J Norton , Douglas Hatch Message-ID: <20160615200407.cTCeXrJDvQ14R0lHRQVt5AQbpi-RLNCnpf2-Ybf43UY@z> On 06/15/2016 03:08 PM, Waiman Long wrote: > On 06/15/2016 01:12 PM, Peter Zijlstra wrote: >> On Wed, Jun 15, 2016 at 09:56:59AM -0700, Davidlohr Bueso wrote: >>> On Tue, 14 Jun 2016, Waiman Long wrote: >>>> +++ b/kernel/locking/osq_lock.c >>>> @@ -115,7 +115,7 @@ bool osq_lock(struct optimistic_spin_queue *lock) >>>> * cmpxchg in an attempt to undo our queueing. >>>> */ >>>> >>>> - while (!READ_ONCE(node->locked)) { >>>> + while (!smp_load_acquire(&node->locked)) { >>> Hmm this being a polling path, that barrier can get pretty expensive >>> and >>> last I checked it was unnecessary: >> I think he'll go rely on it later on. >> >> In any case, its fairly simple to cure, just add >> smp_acquire__after_ctrl_dep() at the end. If we bail because >> need_resched() we don't need the acquire I think. > > Yes, I only need the acquire barrier when the locking is successful. > Thanks for the suggestion. I will make the change accordingly. > BTW, when will the smp_acquire__after_ctrl_dep() patch goes into the tip tree? My patch will have a dependency on that when I make the change. Cheers, Longman