From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: [PATCH v2 2/2] locking/rwsem: Optimize down_read_trylock() Date: Wed, 13 Feb 2019 08:45:44 +0100 Message-ID: <20190213074544.GB62549@gmail.com> References: <1549913486-16799-1-git-send-email-longman@redhat.com> <1549913486-16799-3-git-send-email-longman@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Waiman Long Cc: Linus Torvalds , Peter Zijlstra , Ingo Molnar , Will Deacon , Thomas Gleixner , Linux List Kernel Mailing , linux-alpha@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-hexagon@vger.kernel.org, linux-ia64@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Linux-sh list , sparclinux@vger.kernel.org, linux-xtensa@linux-xtensa.org, linux-arch , the arch/x86 maintainers , Arnd Bergmann , Borislav Petkov , "H. Peter Anvin" , Davidlohr Bueso , Andrew Morton List-Id: linux-arch.vger.kernel.org * Waiman Long wrote: > I looked at the assembly code in arch/x86/include/asm/rwsem.h. For both > trylocks (read & write), the count is read first before attempting to > lock it. We did the same for all trylock functions in other locks. > Depending on how the trylock is used and how contended the lock is, it > may help or hurt performance. Changing down_read_trylock to do an > unconditional cmpxchg will change the performance profile of existing > code. So I would prefer keeping the current code. > > I do notice now that the generic down_write_trylock() code is doing an > unconditional compxchg. So I wonder if we should change it to read the > lock first like other trylocks or just leave it as it is. No, I think we should instead move the other trylocks to the try-for-ownership model as well, like Linus suggested. That's the general assumption we make in locking primitives, that we optimize for the common, expected case - which would be that the trylock succeeds, and I don't see why trylock primitives should be different. In fact I can see more ways for read-for-sharing to perform suboptimally on larger systems. Thanks, Ingo From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f65.google.com ([209.85.128.65]:54798 "EHLO mail-wm1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728291AbfBMHpu (ORCPT ); Wed, 13 Feb 2019 02:45:50 -0500 Date: Wed, 13 Feb 2019 08:45:44 +0100 From: Ingo Molnar Subject: Re: [PATCH v2 2/2] locking/rwsem: Optimize down_read_trylock() Message-ID: <20190213074544.GB62549@gmail.com> References: <1549913486-16799-1-git-send-email-longman@redhat.com> <1549913486-16799-3-git-send-email-longman@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-arch-owner@vger.kernel.org List-ID: To: Waiman Long Cc: Linus Torvalds , Peter Zijlstra , Ingo Molnar , Will Deacon , Thomas Gleixner , Linux List Kernel Mailing , linux-alpha@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-hexagon@vger.kernel.org, linux-ia64@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Linux-sh list , sparclinux@vger.kernel.org, linux-xtensa@linux-xtensa.org, linux-arch , the arch/x86 maintainers , Arnd Bergmann , Borislav Petkov , "H. Peter Anvin" , Davidlohr Bueso , Andrew Morton , Tim Chen Message-ID: <20190213074544.f_Nneih3BYgjHMS5ERXbW6uxoDL4JwpOsMiE0czESrM@z> * Waiman Long wrote: > I looked at the assembly code in arch/x86/include/asm/rwsem.h. For both > trylocks (read & write), the count is read first before attempting to > lock it. We did the same for all trylock functions in other locks. > Depending on how the trylock is used and how contended the lock is, it > may help or hurt performance. Changing down_read_trylock to do an > unconditional cmpxchg will change the performance profile of existing > code. So I would prefer keeping the current code. > > I do notice now that the generic down_write_trylock() code is doing an > unconditional compxchg. So I wonder if we should change it to read the > lock first like other trylocks or just leave it as it is. No, I think we should instead move the other trylocks to the try-for-ownership model as well, like Linus suggested. That's the general assumption we make in locking primitives, that we optimize for the common, expected case - which would be that the trylock succeeds, and I don't see why trylock primitives should be different. In fact I can see more ways for read-for-sharing to perform suboptimally on larger systems. Thanks, Ingo