Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752228AbbBKLIC (ORCPT ); Wed, 11 Feb 2015 06:08:02 -0500 Received: from e23smtp05.au.ibm.com ([202.81.31.147]:42299 "EHLO e23smtp05.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751752AbbBKLH7 (ORCPT ); Wed, 11 Feb 2015 06:07:59 -0500 Message-ID: <54DB384A.2050305@linux.vnet.ibm.com> Date: Wed, 11 Feb 2015 16:38:58 +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: Oleg Nesterov CC: Linus Torvalds , Jeremy Fitzhardinge , Sasha Levin , Davidlohr Bueso , Peter Zijlstra , Thomas Gleixner , Ingo Molnar , Peter Anvin , Konrad Rzeszutek Wilk , Paolo Bonzini , Paul McKenney , Waiman Long , Dave Jones , 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> <54D9CFC7.5020007@linux.vnet.ibm.com> <20150210132634.GA30380@redhat.com> In-Reply-To: <20150210132634.GA30380@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15021111-0017-0000-0000-000000C9AEAD Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1760 Lines: 43 On 02/10/2015 06:56 PM, Oleg Nesterov wrote: > On 02/10, Raghavendra K T wrote: >> >> On 02/10/2015 06:23 AM, Linus Torvalds wrote: >> >>> 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? > > Stupid question... what if we simply move SLOWPATH from .tail to .head? > In this case arch_spin_unlock() could do xadd(tickets.head) and check > the result It is a good idea. Trying this now. > In this case __ticket_check_and_clear_slowpath() really needs to cmpxchg > the whole .head_tail. Plus obviously more boring changes. This needs a > separate patch even _if_ this can work. Correct, but apart from this, before doing xadd in unlock, we would have to make sure lsb bit is cleared so that we can live with 1 bit overflow to tail which is unused. now either or both of head,tail lsb bit may be set after unlock. -- 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/