Re: [PATCH 5/7] xen: sched: get rid of the per domain vCPU list in Credit2

[Andrew Cooper <andrew.cooper3@citrix.com>]

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