[question] bug in cpu_schedule_up?

[question] bug in cpu_schedule_up?

[Kai Huang]

Hi,

I see below code in cpu_schedule_up in xen-unstable hg repository
(xen/common/schedule.c).

    if ( idle_vcpu[cpu] == NULL )
        alloc_vcpu(idle_vcpu[0]->domain, cpu, cpu);
    if ( idle_vcpu[cpu] == NULL )
        return -ENOMEM;

Seems it's a bug? Should be like this?

    if ( idle_vcpu[cpu] == NULL )
        idle_vcpu[cpu] = alloc_vcpu(idle_vcpu[0]->domain, cpu, cpu);
    if ( idle_vcpu[cpu] == NULL )
        return -ENOMEM;


-cody

Re: [question] bug in cpu_schedule_up?

[Ian Campbell]

On Wed, 2012-01-18 at 08:13 +0000, Kai Huang wrote:
> Hi,
> 
> I see below code in cpu_schedule_up in xen-unstable hg repository
> (xen/common/schedule.c).
> 
>     if ( idle_vcpu[cpu] == NULL )
>         alloc_vcpu(idle_vcpu[0]->domain, cpu, cpu);
>     if ( idle_vcpu[cpu] == NULL )
>         return -ENOMEM;
> 
> Seems it's a bug? Should be like this?
> 
>     if ( idle_vcpu[cpu] == NULL )
>         idle_vcpu[cpu] = alloc_vcpu(idle_vcpu[0]->domain, cpu, cpu);
>     if ( idle_vcpu[cpu] == NULL )
>         return -ENOMEM;

alloc_vcpu will set idle_vcpu[0]->domain->vcpu[cpu] to the newly
allocated vcpu. idle_vcpu[0]->domain == idle_domain and
idle_vcpu[0]->domain->vcpu == idle_vcpu (both are by construction in
scheduler_init). Therefore idle_vcpu[cpu] is already being set inside
alloc_vcpu.

The return value of alloc_vcpu is to save code which wants a local
handle on the newly allocated vcpu to perform further setup from doing
the lookup itself.

Ian.

Re: [question] bug in cpu_schedule_up?

[Ian Campbell]

(putting list back)

On Wed, 2012-01-18 at 09:17 +0000, Kai Huang wrote:
> On Wed, Jan 18, 2012 at 4:38 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Wed, 2012-01-18 at 08:13 +0000, Kai Huang wrote:
> >> Hi,
> >>
> >> I see below code in cpu_schedule_up in xen-unstable hg repository
> >> (xen/common/schedule.c).
> >>
> >>     if ( idle_vcpu[cpu] == NULL )
> >>         alloc_vcpu(idle_vcpu[0]->domain, cpu, cpu);
> >>     if ( idle_vcpu[cpu] == NULL )
> >>         return -ENOMEM;
> >>
> >> Seems it's a bug? Should be like this?
> >>
> >>     if ( idle_vcpu[cpu] == NULL )
> >>         idle_vcpu[cpu] = alloc_vcpu(idle_vcpu[0]->domain, cpu, cpu);
> >>     if ( idle_vcpu[cpu] == NULL )
> >>         return -ENOMEM;
> >
> > alloc_vcpu will set idle_vcpu[0]->domain->vcpu[cpu] to the newly
> > allocated vcpu. idle_vcpu[0]->domain == idle_domain and
> > idle_vcpu[0]->domain->vcpu == idle_vcpu (both are by construction in
> > scheduler_init). Therefore idle_vcpu[cpu] is already being set inside
> > alloc_vcpu.
> 
> Yes you are right. I finally find out this pointer chain. My mistake. Thanks.
> 
> But this brings me questions about vcpu management, specifically:
> 
> 1) When will the vcpu be created exactly? Seems vcpus are created on demand?

Either the toolstack or the domain itself (up to a limit set by the
toolstack) will create them via hypercalls, see VCPUOP_* (initialise and
up in particular) and use grep/TAGS/etc to follow the code from
alloc_vcpus and you should be able to see this and answer your other
questions too.

> 2) The vcpu created at beginning belongs to idle_domain, right?

Yes. The hypervisor will also build domain 0 and create vcpus for it.
All the other domains are created by the toolstack.

> When will the vcpus be assigned to other domain?

When that domain is created and the VCPUs are created/initialised they
will be assigned to the domain.

> 3) I geuss the binding of vcpu to physical cpu will be dynamically
> changed when scheduling, which means changing the binding scheduler
> specific, is this correct?

I'm not sure but I think that the individual schedulers just nominate
which VCPU to run on each PCPU while the core scheduler takes care of
the associated book keeping with actually moving stuff around. I could
be mistaken here though.

Ian.