Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754252AbbBJJaB (ORCPT ); Tue, 10 Feb 2015 04:30:01 -0500 Received: from e23smtp01.au.ibm.com ([202.81.31.143]:57698 "EHLO e23smtp01.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754117AbbBJJ35 (ORCPT ); Tue, 10 Feb 2015 04:29:57 -0500 Message-ID: <54D9CFC7.5020007@linux.vnet.ibm.com> Date: Tue, 10 Feb 2015 15:00:47 +0530 From: Raghavendra K T Organization: IBM User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Linus Torvalds , Jeremy Fitzhardinge , Sasha Levin , Davidlohr Bueso CC: Peter Zijlstra , Thomas Gleixner , Ingo Molnar , Peter Anvin , Konrad Rzeszutek Wilk , Paolo Bonzini , Paul McKenney , Waiman Long , Dave Jones , Oleg Nesterov , the arch/x86 maintainers , Paul Gortmaker , Andi Kleen , Jason Wang , Linux Kernel Mailing List , KVM list , virtualization , xen-devel@lists.xenproject.org, Rik van Riel , Christian Borntraeger , Andrew Morton , Andrey Ryabinin Subject: Re: [PATCH] x86 spinlock: Fix memory corruption on completing completions References: <1423234148-13886-1-git-send-email-raghavendra.kt@linux.vnet.ibm.com> <54D7D19B.1000103@goop.org> <54D87F1E.9060307@linux.vnet.ibm.com> <20150209120227.GT21418@twins.programming.kicks-ass.net> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15021009-1618-0000-0000-00000195343B Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2627 Lines: 69 On 02/10/2015 06:23 AM, Linus Torvalds wrote: > On Mon, Feb 9, 2015 at 4:02 AM, Peter Zijlstra wrote: >> On Mon, Feb 09, 2015 at 03:04:22PM +0530, Raghavendra K T wrote: >>> So we have 3 choices, >>> 1. xadd >>> 2. continue with current approach. >>> 3. a read before unlock and also after that. >> >> For the truly paranoid we have probe_kernel_address(), suppose the lock >> was in module space and the module just got unloaded under us. > > That's much too expensive. > > The xadd shouldn't be noticeably more expensive than the current > "add_smp()". Yes, "lock xadd" used to be several cycles slower than > just "lock add" on some early cores, but I think these days it's down > to a single-cycle difference, which is not really different from doing > a separate load after the add. > > The real problem with xadd used to be that we always had to do magic > special-casing for i386, but that's one of the reasons we dropped > support for original 80386. > > So I think Raghavendra's last version (which hopefully fixes the > lockup problem that Sasha reported) together with changing that V2 did pass the stress, but getting confirmation Sasha would help. > add_smp(&lock->tickets.head, TICKET_LOCK_INC); > if (READ_ONCE(lock->tickets.tail) & TICKET_SLOWPATH_FLAG) .. > > into something like > > val = xadd((&lock->ticket.head_tail, TICKET_LOCK_INC << TICKET_SHIFT); > if (unlikely(val & TICKET_SLOWPATH_FLAG)) ... > > would be the right thing to do. Somebody should just check that I got > that shift right, and that the tail is in the high bytes (head really > needs to be high to work, if it's in the low byte(s) the xadd would > overflow from head into tail which would be wrong). Unfortunately xadd could result in head overflow as tail is high. The other option was repeated cmpxchg which is bad I believe. Any suggestions? ( I have the V3 with Oleg's suggestion and performance numbers but without this getting resolved, It will be one unnecessary iteration). How about getting rid off SLOW_PATH_FLAG in spinlock (i.e. use it only as hint for paravirt), but do unlock_kick whenever we see that (tail-head) > TICKET_LOCK_INC?. (but this also may need cmpxchg in loop in unlock but we will be able to get rid of clear slowpath logic) Only problem is we may do unnecessary kicks even in 1x load. -- 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/