Re: [Xen-devel] [PATCH 4/9] xen: sched: add .init_pdata hook to the scheduler interface

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Thu, 2015-10-01 at 02:17 -0600, Jan Beulich wrote:
> > > > On 29.09.15 at 18:55, <dario.faggioli@xxxxxxxxxx> wrote:
> > This happens because, right now, the scheduler of the
> > target pool remaps the runqueue lock during (rt_)alloc_pdata,
> > which is called at the very beginning of schedule_cpu_switch().
> > Credit2's csched2_free_pdata(), however, wants to find the spin
> > lock the same way it was put by csched2_alloc_pdata(), and
> > explodes, it not being the case.
> > 
> > This patch also fixes this as, now, spin lock remapping
> > happens in the init_pdata hook of the target scheduler for
> > the pCPU, which we can easily make sure it is ivoked *after*
> > the free_pdata hook of the old scheduler (which is exactly
> > what is done in this patch), without needing to restructure
> > schedule_cpu_switch().
> 
> Hmm, while I can see this to be okay for the
> cpupool_unassign_cpu_helper() case (due to the call to
> cpu_disable_scheduler() up front) as well as for the
> cpupool_cpu_add() one (I take it that the CPU can't be
> scheduled on in that case), I don't see how this would be
> safe for the XEN_SYSCTL_CPUPOOL_OP_ADDCPU case: How is
> scheduling activity being prevented to happen after the new
> ppriv got installed, but before it gets initialized via the
> init_pdata
> hook? Or how would scheduling activity be safe with an allocated
> but not yet initialized ppriv?
> 
Not sure I'm getting, but let me try.

XEN_SYSCTL_CPUPOOL_OP_ADDCPU is what adds a CPU to a pool. If we are
adding a CPU to a pool, it means the CPU is currently free, i.e.,
outside of any pool. This, in turn, means no pool has the CPU's bit set
in its cpupool->cpu_valid mask, which is what we check, within the
scheduler, to see where we can schedule the vcpus subject to a
particular scheduler.

If no pool has that bit set in its mask, nothing is being scheduled on
that CPU. (Actually, making sure that that is really the case, has been
the subject of some patches I sent back in July.)

Ok, now XEN_SYSCTL_CPUPOOL_OP_ADDCPU calls cpupool_assign_cpu_locked(),
which calls schedule_cpu_switch(cpu --> c) and, *only* *after* *that*,
does the following:

    cpumask_clear_cpu(cpu, &cpupool_free_cpus);
    ...
    cpumask_set_cpu(cpu, c->cpu_valid);

So, still assuming that I got your question right, the answer is:
scheduling activity was stopped already, and is restarted only after
both allocation and initialization have happened.

Let me know what you think.

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: signature.asc
Description: This is a digitally signed message part

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