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 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=WSLNpQeKH85LdvlZ78tn7b/TaWGoRsly3MpPfDxVU28=; b=n0Uu+1spwh6BP8F080TtY1cvdPiS5MomeYgaSXLX0ODIFfwbvTDCFCySsQcLxVKGmM dw/96VmrPhtvCuR7/3aha/Y+YTXZYmokABimDEcEnAeR0hD3UDlcQCUCKdmikIZpJm7D V81pl3f8hRpJxAFdazAHEXOg7tIHMAM9CYE2zVy7dMTLwKTYDzwRNPhxrhdAIO52PsDj w/7l9Pu0cYzoafONIE5xszt/iaFRaoZh+SEBV/1dLccfZHIgA35DSaB0okBf0jyDgwDz kUXx6/grQqC0RNDfrI1CRbMCF2x8FbFq5HpRq/FEdLVE5s+21PZZUKFE+EKOUMwCUXXD hTkw== Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 * 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