Re: [PATCH v3 2/6] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2585 bytes --]

On Wed, 2015-11-04 at 10:01 -0500, Meng Xu wrote:
> 2015-11-04 9:12 GMT-05:00 Dario Faggioli <dario.faggioli@citrix.com>:
> > Just FTR (and for next time :-D), is the above something that can
> > be
> > interpreted as a 'Reviewed-by: Meng Xu <xxx>' ?  If no (e.g.,
> > because
> > you haven't looking thoroughly enough to feel confident to express
> > it),
> > then fine, I was just asking.
> Thank you very much for explaining this for me. :-) 
> 
> I feel confident about the changes for RTDS scheduler. 
>
Ok.

> I'm not so confident about the change in the schedule.c. To be
> specific, this patch removes insert_vcpu in schedule_cpu_switch() in
> schedule.c; 
>
It removes the attempt of inserting the idle vCPU in the runqueue of
the scheduler of the target cpupool for the pCPU.

More specifically, this line:

  SCHED_OP(new_ops, insert_vcpu, idle);

If we look at the various ways in which insert_vcpu is implemented, we
have:

csched_vcpu_insert:

    if ( !__vcpu_on_runq(svc) && vcpu_runnable(vc) && !vc->is_running )
        __runq_insert(vc->processor, svc);

but the pCPU being switched is free, i.e., it is not in any cpupool,
and it is idling. So, the idle vCPU is running, and the condition above
is false, which means __runq_insert() is not really called.

csched2_vcpu_insert:

    if ( ! is_idle_vcpu(vc) )
    {
     ...
    }

so trying to insert the idle vCPU is actually a nop.

rt_vcpu_insert:

    if ( is_idle_vcpu(vc) )
        return;

a nop again.

My point being that this patch actually removes nothing but a bunch of
if()-s, with no effect at all.

> I'm not so sure if it is ok to insert_vcpu when a domain is moved.

>
Hopefully, I addressed your doubts.

BTW, we're not talking about a domain being moved between pools. That
is done with another function. schedule_cpu_switch() is about taking a
pCPU off from a cpupool, or assigning a pCPU to cpupool.

> (Next time, I will stand out and ask although it may be a stupid
> question. ;-) )
> 
Sure! And this does not look a stupid question to me. :-)

> So as to this patch, I will say:
> As far as the RTDS scheduler is concerned: Reviewed-by: Meng Xu <
> mengxu@cis.upenn.edu>
>  
Ok, I haven't sent v4 yet, so I'll apply it there. Thanks.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel