From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 5/7] xen: sched: get rid of the per domain vCPU list in Credit2 Date: Thu, 8 Oct 2015 16:39:23 +0100 Message-ID: <56168E2B.7000908@citrix.com> References: <20151008124027.12522.42552.stgit@Solace.station> <20151008125305.12522.60745.stgit@Solace.station> <56167628.6050400@citrix.com> <1444318365.22254.29.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZkDI0-000159-1W for xen-devel@lists.xenproject.org; Thu, 08 Oct 2015 15:39:28 +0000 In-Reply-To: <1444318365.22254.29.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 , xen-devel@lists.xenproject.org Cc: George Dunlap List-Id: xen-devel@lists.xenproject.org On 08/10/15 16:32, Dario Faggioli wrote: > On Thu, 2015-10-08 at 14:56 +0100, Andrew Cooper wrote: >> On 08/10/15 13:53, Dario Faggioli wrote: >>> @@ -1443,7 +1433,7 @@ csched2_dom_cntl( >>> >>> if ( op->u.credit2.weight != 0 ) >>> { >>> - struct list_head *iter; >>> + struct vcpu *vc; >> Any chance of starting to align on the more common practice of just v >> for a vcpu? >> > I see. I'm a bit split, though. It's v in schedule.c, but it's > **always** (with only 2 exceptions) vc in sched_*.c. > > I know that, if we want to align, we need to start from somewhere, but > OTOH, consistency is rather helpful when reading this code. > > I'll think about it... > >>> @@ -1539,9 +1528,7 @@ csched2_free_domdata(const struct scheduler >>> *ops, void *data) >>> static void >>> csched2_dom_destroy(const struct scheduler *ops, struct domain >>> *dom) >>> { >>> - struct csched2_dom *sdom = CSCHED2_DOM(dom); >>> - >>> - BUG_ON(!list_empty(&sdom->vcpu)); >>> + BUG_ON(CSCHED2_DOM(dom)->nr_vcpus != 0); >> This is a latent bug (excuse the pun) which can be triggered by >> userspace. There is no guarantee or requirement that a domain >> registered with a scheduler has vcpus. >> > Mmm... I think the original check wanted to catch cases where a domain > (or, at least, its scheduling related bits) is being destroyed with > vcpus still on. Oops sorry - I got the condition inverted when thinking about it. Also, this is a csched2_dom rather than a struct domain. I rescind my query. ~Andrew