Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751917AbbGJBKN (ORCPT ); Thu, 9 Jul 2015 21:10:13 -0400 Received: from g2t2354.austin.hp.com ([15.217.128.53]:44409 "EHLO g2t2354.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751346AbbGJBKJ (ORCPT ); Thu, 9 Jul 2015 21:10:09 -0400 Message-ID: <559F1B6C.7040706@hp.com> Date: Thu, 09 Jul 2015 21:10:04 -0400 From: Waiman Long User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130109 Thunderbird/10.0.12 MIME-Version: 1.0 To: Davidlohr Bueso CC: Peter Zijlstra , Ingo Molnar , Arnd Bergmann , Thomas Gleixner , linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, Will Deacon , Scott J Norton , Douglas Hatch Subject: Re: [PATCH v2 1/2] locking/qrwlock: Reduce reader/writer to reader lock transfer latency References: <1436459543-29126-1-git-send-email-Waiman.Long@hp.com> <1436459543-29126-2-git-send-email-Waiman.Long@hp.com> <1436475158.12255.119.camel@stgolabs.net> In-Reply-To: <1436475158.12255.119.camel@stgolabs.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3071 Lines: 71 On 07/09/2015 04:52 PM, Davidlohr Bueso wrote: > On Thu, 2015-07-09 at 12:32 -0400, Waiman Long wrote: >> This patch eliminates that waiting. It also has the side effect >> of reducing the chance of writer lock stealing and improving the >> fairness of the lock. Using a locking microbenchmark, a 10-threads 5M >> locking loop of mostly readers (RW ratio = 10,000:1) has the following >> performance numbers in a Haswell-EX box: >> >> Kernel Locking Rate (Kops/s) >> ------ --------------------- >> 4.1.1 15,063,081 >> Patched 4.1.1 17,241,552 > In any case, for such read-mostly scenarios, you'd probably want to be > using rcu ;-). Yes, I agree:-) >> Signed-off-by: Waiman Long >> --- >> kernel/locking/qrwlock.c | 12 ++++-------- >> 1 files changed, 4 insertions(+), 8 deletions(-) >> >> diff --git a/kernel/locking/qrwlock.c b/kernel/locking/qrwlock.c >> index d9c36c5..6a7a3b8 100644 >> --- a/kernel/locking/qrwlock.c >> +++ b/kernel/locking/qrwlock.c >> @@ -88,15 +88,11 @@ void queued_read_lock_slowpath(struct qrwlock *lock, u32 cnts) >> arch_spin_lock(&lock->lock); >> >> /* >> - * At the head of the wait queue now, wait until the writer state >> - * goes to 0 and then try to increment the reader count and get >> - * the lock. It is possible that an incoming writer may steal the >> - * lock in the interim, so it is necessary to check the writer byte >> - * to make sure that the write lock isn't taken. >> + * At the head of the wait queue now, increment the reader count >> + * and wait until the writer, if it has the lock, has gone away. >> + * At ths > ^^ this > >> stage, it is not possible for a writer to remain in the >> + * waiting state (_QW_WAITING). So there won't be any deadlock. > Because the writer setting _QW_WAITING is done in the slowpath, > serialized with the qrwlock->lock, right? _QW_WAITING can only be set when the writer is at the queue head, and it will become _QW_LOCKED when it gets the lock. When a reader becomes queue head, the writer byte can either be 0 or _QW_LOCKED, but it can never be _QW_WAITING. >> */ >> - while (atomic_read(&lock->cnts)& _QW_WMASK) >> - cpu_relax_lowlatency(); >> - >> cnts = atomic_add_return(_QR_BIAS,&lock->cnts) - _QR_BIAS; > Nit: since 'cnts' is now only the original value of lock->cnts before > adding _QR_BIAS, could we rename it to 'prev_cnts' (or something)? -- > iirc you removed the need for the variable when in interrupt context. The subtraction sign is there to simulate an xadd instruction. Without that, the generated code will have an additional add instruction. Yes, it is kind of a hack. It will be changed later on when other architectures start using qrwlock. Cheers, Longman -- 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/