From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 3/9] xen: sched: make locking for {insert, remove}_vcpu consistent Date: Fri, 9 Oct 2015 14:05:09 +0100 Message-ID: <5617BB85.3090703@citrix.com> References: <20150929164726.17589.96920.stgit@Solace.station> <20150929165549.17589.76223.stgit@Solace.station> <560ACAD5.8040405@citrix.com> <56168479.60703@citrix.com> <561689B6.4020306@citrix.com> <1444336793.22254.72.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZkXMJ-0007fY-8P for xen-devel@lists.xenproject.org; Fri, 09 Oct 2015 13:05:15 +0000 In-Reply-To: <1444336793.22254.72.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli , George Dunlap , xen-devel@lists.xenproject.org Cc: George Dunlap , Meng Xu List-Id: xen-devel@lists.xenproject.org On 08/10/15 21:39, Dario Faggioli wrote: > On Thu, 2015-10-08 at 16:20 +0100, Andrew Cooper wrote: >> On 08/10/15 15:58, George Dunlap wrote: >>> Generic scheduling code is called from interrupt contexts -- >>> namely, >>> vcpu_wake() >> There are a lot of codepaths, but I cant see one which is definitely >> called with interrupts disables. (OTOH, I can see several where >> interrupts are definitely enabled). >> > Sorry, it's certainly me, but I don't think I understand this. > > AFAICT, you are saying that you fail to find in the code, situations > the scheduler code is invoked, with interrupts already disabled, is > this correct? In particular "definitely called with interrupt disabled" > is what confuses me... :-/ Given a brief look at the code, I can't spot a codepath where it is obvious that interrupts are disabled. > > Also (assuming what I just said is what you meant), are you referring > "just" to schedule(), or even to other scheduler's code, which also > disables interrupt when taking the lock(s) it needs, like, e.g., > vcpu_wake() as George said? I was looking on callchains involving vcpu_wake(). > >>> , which for the credit scheduler wants to put things on the >>> runqueue. Lock taken in interrupt context => interrupts must be >>> disabled whenever taking the lock, yes? >> Correct, which is the purpose of the ASSERT()s in the _irq() and >> _irqsave() variants. >> > What ASSERT()? I don't find any assert in _spin_lock_irqsave() (which > thing makes sense to me): Ah yes - _irqsave() wouldn't want an assertion. ~Andrew