From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754244Ab1A0XyE (ORCPT ); Thu, 27 Jan 2011 18:54:04 -0500 Received: from claw.goop.org ([74.207.240.146]:54029 "EHLO claw.goop.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752762Ab1A0XyC (ORCPT ); Thu, 27 Jan 2011 18:54:02 -0500 Message-ID: <4D420597.9000305@goop.org> Date: Thu, 27 Jan 2011 15:53:59 -0800 From: Jeremy Fitzhardinge User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Lightning/1.0b3pre Thunderbird/3.1.7 MIME-Version: 1.0 To: Nick Piggin CC: Peter Zijlstra , "H. Peter Anvin" , Ingo Molnar , the arch/x86 maintainers , Linux Kernel Mailing List , Nick Piggin , Jeremy Fitzhardinge Subject: Re: [PATCH 6/6] x86/ticketlock: make __ticket_spin_trylock common References: <4086953399116e4d9a14f9fa1aa8cf33aaef753d.1295909909.git.jeremy.fitzhardinge@citrix.com> <4D3E2A80.1000203@goop.org> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/24/2011 05:58 PM, Nick Piggin wrote: >> >> The C version can't take advantage of the fact that the cmpxchg directly >> sets the flags, so it ends up re-comparing the old and swapped-out >> values to set the return. And it doesn't re-use the same sete to set >> the return value in the quick failed-to-acquire path. > Hm. Adding a "cmpxchg_flag" which does its own sete and returns a boolean "success" flag, the whole thing goes to: __ticket_spin_trylock: movw (%rdi), %ax # lock_2(D)->D.5950.tickets, old xorl %edx, %edx # D.13949 movzbl %ah, %ecx # old, tmp69 cmpb %al, %cl # old, tmp69 jne .L5 #, leal 256(%rax), %edx #, D.13951 lock; cmpxchgw %dx,(%rdi); sete %al # D.13951,* lock, __ret movzbl %al, %edx # __ret, D.13949 .L5: movl %edx, %eax # D.13949, ret The eax/edx shuffle is a bit unfortunate I can't see it hurting very much. >> It might be worth having a generic cmpxchg() variant which returns a >> succeed/fail flag rather than the fetched value, to avoid comparison in >> this case - since many (most?) cmpxchg() callers end up doing that >> comparison. >> >> How performance critical is trylock? I guess the ones in fs/dcache.c >> are the ones looming large in your mind. > Well they are on on the reclaim/free path rather than the _hottest_ > paths, but yes they are performance critical. I think that code looks pretty reasonable. J