Re: [PATCH v2 2/2] sched_credit2.c: runqueue_per_core code

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

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

On Thu, 2015-03-19 at 10:50 +0000, Jan Beulich wrote:
> >>> On 19.03.15 at 11:03, <dario.faggioli@citrix.com> wrote:
> > OTOH, CPU_STARTING callbacks run:
> >  - on the cpu being brought up;
> >  - with interrupt disabled (see how the call to local_irq_enable(), in
> >    start_secondary(), is *after* the invocation of
> >    notify_cpu_starting()).
> > 
> > Here we are. And the reason why things works ok in Credit2, is that
> > csched2_alloc_pdata() doesn't really allocate anything! In fact, in
> > general, handling alloc_pdata() during CPU_STARTING would mean that we
> > can't allocate any memory which, given the name of the function, would
> > look rather odd. :-)
> > 
> > Nevertheless I see the value of doing so, and hence I think what we
> > could do would be to introduce a new hook in the scheduler interface,
> > called .init_pdata or .init_pcpu, and, in sched_*.c, split the
> > allocation and the initialization parts. The former will be handled
> > during CPU_UP_PREPARE, when allocation is possible, the latter during
> > CPU_STARTING, when we have more info available to perform actual
> > initializations.
> 
> Another alternative would be a new CPU_ALIVE (name subject to
> change) notification after interrupts got enabled. 
>
I'd be certainly up to try this too, if it is considered better.

BTW, according to my greps, there aren't much users of the CPU_STARTING
callback... Credit2 really seems to be the only one:

[dario@Solace xen.git] $ grep CPU_STARTING xen/* -R
xen/common/sched_credit2.c:    case CPU_STARTING:
xen/common/cpu.c:        &cpu_chain, CPU_STARTING, hcpu, NULL);
xen/include/xen/cpu.h: *  CPU_UP_PREPARE -> CPU_STARTING -> CPU_ONLINE -- successful CPU up
xen/include/xen/cpu.h:/* CPU_STARTING: CPU nearly online. Runs on new CPU, irqs still disabled. */
xen/include/xen/cpu.h:#define CPU_STARTING     (0x0003 | NOTIFY_FORWARD)
xen/include/xen/cpu.h:/* From arch code, send CPU_STARTING notification. */

With my proposal, it will still be used by the scheduler code only,
although (potentially) by all schedulers, not by just Credit2.

If we bring in CPU_ALIVE, and we move _all_ scheduler related pCPU init
code (both allocation and actual initialization) in there, CPU_STARTING
will become unused.

I'm not sure this means we could then remove it, or either move it below
IRQ enabling and just use it (instead of adding another). In fact, it
perhaps would still be good to have the chance, in future, to execute
code on the cpu with IRQ disabled, or not? In any case, I thought this
was worth mentioning.

Regards,
Dario

[-- 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