Re: [Xen-devel] [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu blocking list

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Dario Faggioli [mailto:dario.faggioli@xxxxxxxxxx]
> Sent: Friday, June 24, 2016 3:23 PM
> To: Wu, Feng <feng.wu@xxxxxxxxx>; Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Tian, Kevin <kevin.tian@xxxxxxxxx>; keir@xxxxxxx;
> george.dunlap@xxxxxxxxxxxxx; andrew.cooper3@xxxxxxxxxx; xen-
> devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH 0/3] VMX: Properly handle pi descriptor and
> per-cpu blocking list
> 
> On Fri, 2016-06-24 at 06:11 +0000, Wu, Feng wrote:
> > > -----Original Message-----
> > > From: Dario Faggioli [mailto:dario.faggioli@xxxxxxxxxx]
> > > No, because we call cpu_disable_scheduler() from __cpu_disable(),
> > > only
> > > when system state is SYS_STATE_suspend already, and hence we take
> > > the
> > > then branch of the 'if', which does never return an error.
> > Thanks for the elaboration. I find __cpu_disable() can be called with
> > system state not being SYS_STATE_suspend. Here is my experiment:
> >
> > 1. Launch a guest and pin vCPU 3 to pCPU 3 (which makes the
> > experiment simpler)
> > 2. offline pCPU 3 via "xen-hptool cpu-offline 3"
> >
> Ah, yes, of course. I should have included cpu-hot(un)plug in my
> analysis in the first place, sorry for not doing so.
> 
> > The call path of the above steps is:
> > arch_do_sysctl()
> >   -> cpu_down_helper()
> >     -> cpu_down()
> >       -> take_cpu_down()
> >         -> __cpu_disable()
> >           -> cpu_disable_scheduler() (enter the 'else'  part)
> >
> Right, and the important part is this one:
> 
> > int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu)
> > {
> >
> > ......
> >
> > point 1
> >
> >     for_each_cpu ( i, &allbutself )
> >         tasklet_schedule_on_cpu(&per_cpu(stopmachine_tasklet, i), i);
> >
> > point 2
> > ......
> >
> > }
> >
> > at point 1 above,
> > vCPU3->runstate.state: 0, vCPU3->is_running: 1
> > while at point 2 above:
> > vCPU3->runstate.state: 1, vCPU3->is_running: 0
> >
> This is exactly as you describe. cpu hotplug is done in stop machine
> context. Check the comment close to the definition of stop_machine_run:
> 
> /**
>  * stop_machine_run: freeze the machine on all CPUs and run this function
>  * @fn: the function to run
>  * @data: the data ptr for the @fn()
>  * @cpu: the cpu to run @fn() on (or all, if @cpu == NR_CPUS).
>  *
>  * Description: This causes every other cpu to enter a safe point, with
>  * each of which disables interrupts, and finally interrupts are disabled
>  * on the current CPU.  The result is that none is holding a spinlock
>  * or inside any other preempt-disabled region when @fn() runs.
>  *
>  * This can be thought of as a very heavy write lock, equivalent to
>  * grabbing every spinlock in the kernel. */
> 
> As you discovered yourself, this is achieved by forcing the execution
> of a tasklet on all the pcpus, which include pCPU 3 of your example.
> 
> So, vCPU 3 was running, but then some called stop_machine_run(), which
> causes the descheduling of vCPU 3, and the execution of the stopmachine
> tasklet.

Thanks for your replay. Yes, I think this is point. Here descheduling of vCPU3
happens, and the reason we will choose the tasklet as the next running
unit for sure (not choosing another vCPU or vCPU3 itself as the next
running unit) is because tasklet will overrides all other choose as
stated in csched_schedule() as below, right?

static struct task_slice
csched_schedule(
    const struct scheduler *ops, s_time_t now, bool_t tasklet_work_scheduled) 
{
......

    snext = __runq_elem(runq->next);
    ret.migrated = 0;

    /* Tasklet work (which runs in idle VCPU context) overrides all else. */
    if ( tasklet_work_scheduled )
    {
        TRACE_0D(TRC_CSCHED_SCHED_TASKLET);
        snext = CSCHED_VCPU(idle_vcpu[cpu]);
        snext->pri = CSCHED_PRI_TS_BOOST;
    }


......
}

Thanks,
Feng
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel