From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] Fix locking bug in vcpu_migrate Date: Fri, 22 Apr 2011 19:43:24 +0100 Message-ID: References: <4DB1C7A3.80104@nuclearfallout.net> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4DB1C7A3.80104@nuclearfallout.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: John Weekes Cc: "xen-devel@lists.xensource.com" , George Dunlap List-Id: xen-devel@lists.xenproject.org On 22/04/2011 19:23, "John Weekes" wrote: > >> Nice that empirical evidence supports the patch, however, I'm being dense >> and don't understand why order of lock release matters. > > I was taught long ago to release them in order, but as I think about it, > I agree with you that there doesn't seem to be a scenario when the > release would matter. > > It's odd that it seemed to lead to such a big difference for me, then. > I'll do some further tests -- maybe I changed something else to cause > the behavior, or the problem is more random than I thought and just > hasn't occurred for me yet in all the new tests. Thanks! -- Keir >> perhaps you've merely perturbed a fragile pos code that's >> broken in some other way. > > That's entirely possible. > >> Also the last hunk of your patch is broken -- in the final else clause you >> call spin_unlock_irqrestore on the wrong lock. This is very definitely a >> bug, as irqs should never be enabled while any schedule_lock is held. > > Definitely. I had fixed that here but sent an old version of the patch > -- a boneheaded mistake. > > -John