Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756027AbbBLPBg (ORCPT ); Thu, 12 Feb 2015 10:01:36 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:41985 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755416AbbBLPBf (ORCPT ); Thu, 12 Feb 2015 10:01:35 -0500 Date: Thu, 12 Feb 2015 16:00:59 +0100 From: Peter Zijlstra To: Raghavendra K T Cc: tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, torvalds@linux-foundation.org, konrad.wilk@oracle.com, pbonzini@redhat.com, paulmck@linux.vnet.ibm.com, waiman.long@hp.com, davej@redhat.com, oleg@redhat.com, x86@kernel.org, jeremy@goop.org, paul.gortmaker@windriver.com, ak@linux.intel.com, jasowang@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, xen-devel@lists.xenproject.org, riel@redhat.com, borntraeger@de.ibm.com, akpm@linux-foundation.org, a.ryabinin@samsung.com, sasha.levin@oracle.com, dave@stgolabs.net Subject: Re: [PATCH V3] x86 spinlock: Fix memory corruption on completing completions Message-ID: <20150212150059.GA23123@twins.programming.kicks-ass.net> References: <1423741647-2156-1-git-send-email-raghavendra.kt@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1423741647-2156-1-git-send-email-raghavendra.kt@linux.vnet.ibm.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2067 Lines: 47 On Thu, Feb 12, 2015 at 05:17:27PM +0530, Raghavendra K T wrote: > Paravirt spinlock clears slowpath flag after doing unlock. > As explained by Linus currently it does: > prev = *lock; > add_smp(&lock->tickets.head, TICKET_LOCK_INC); > > /* add_smp() is a full mb() */ > > if (unlikely(lock->tickets.tail & TICKET_SLOWPATH_FLAG)) > __ticket_unlock_slowpath(lock, prev); > > > which is *exactly* the kind of things you cannot do with spinlocks, > because after you've done the "add_smp()" and released the spinlock > for the fast-path, you can't access the spinlock any more. Exactly > because a fast-path lock might come in, and release the whole data > structure. > > Linus suggested that we should not do any writes to lock after unlock(), > and we can move slowpath clearing to fastpath lock. > > So this patch implements the fix with: > 1. Moving slowpath flag to head (Oleg). > 2. use xadd to avoid read/write after unlock that checks the need for > unlock_kick (Linus). Maybe spend a few more words explaining these things; something like: Unlocked locks don't care about the slowpath flag; therefore we can keep it set after the last unlock, as long as we clear it again on the first (try)lock -- this removes the write after unlock. By moving the slowpath flag from the tail to the head ticket we avoid the need to access both the head and tail tickets on unlock. We can further avoid the need for a read-after-release by using xadd; the prev head value will include the slowpath flag and indicate if we need to do PV kicking of suspended spinners -- on modern chips xadd isn't (much) more expensive than an add + load. Its 'obvious' now, but maybe not so much after we've all not looked at this for a few months. -- 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/