From mboxrd@z Thu Jan 1 00:00:00 1970 From: Waiman Long Subject: Re: [PATCH v2 2/2] locking/rwsem: Optimize down_read_trylock() Date: Wed, 13 Feb 2019 10:33:58 -0500 Message-ID: References: <1549913486-16799-1-git-send-email-longman@redhat.com> <1549913486-16799-3-git-send-email-longman@redhat.com> <20190213074544.GB62549@gmail.com> Mime-Version: 1.0 Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <20190213074544.GB62549@gmail.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" To: Ingo Molnar 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 On 02/13/2019 02:45 AM, Ingo Molnar wrote: > * 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. I don't mind changing to the try-for-ownership model for rwsem and mutex. I do have some concern to do that for spinlock. Some of the lock slowpath code do optimistic trylock. Making them unconditional cmpxchg will impact lock contention performance. I will update this rwsem patch to make the change while I am working on it. For other locks, I will suggest we go slow and carefully evaluate the performance implication before we make the changes. Cheers, Longman