| Message ID | 20160318190408.8117.13604.stgit@Solace.station (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18/03/16 20:04, Dario Faggioli wrote: > with the purpose of decoupling the allocation phase and > the initialization one, for per-pCPU data of the schedulers. > > This makes it possible to perform the initialization later > in the pCPU bringup/assignement process, when more information > (for instance, the host CPU topology) are available. This, > for now, is important only for Credit2, but it can well be > useful to other schedulers. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> > --- > Cc: George Dunlap <george.dunlap@eu.citrix.com> > Cc: Juergen Gross <jgross@suse.com> > --- > Changes from v1: > * in schedule_cpu_switch(), call to init_pdata() moved up, > close to the call to alloc_pdata() (for consistency with > other call sites) and prototype slightly changed. > --- > During v1 review, it was agreed to add ASSERTS() and comments > to clarify the use of schedule_cpu_switch(). This can't be > found here, but only because it has happened in another patch. > --- > xen/common/schedule.c | 7 +++++++ > xen/include/xen/sched-if.h | 1 + > 2 files changed, 8 insertions(+) > > diff --git a/xen/common/schedule.c b/xen/common/schedule.c > index e57b659..0627eb5 100644 > --- a/xen/common/schedule.c > +++ b/xen/common/schedule.c > @@ -1517,10 +1517,15 @@ static int cpu_schedule_callback( > struct notifier_block *nfb, unsigned long action, void *hcpu) > { > unsigned int cpu = (unsigned long)hcpu; > + struct scheduler *sched = per_cpu(scheduler, cpu); > + struct schedule_data *sd = &per_cpu(schedule_data, cpu); > int rc = 0; > > switch ( action ) > { > + case CPU_STARTING: > + SCHED_OP(sched, init_pdata, sd->sched_priv, cpu); > + break; > case CPU_UP_PREPARE: > rc = cpu_schedule_up(cpu); > break; > @@ -1597,6 +1602,7 @@ void __init scheduler_init(void) > if ( ops.alloc_pdata && > !(this_cpu(schedule_data).sched_priv = ops.alloc_pdata(&ops, 0)) ) > BUG(); > + SCHED_OP(&ops, init_pdata, this_cpu(schedule_data).sched_priv, 0); Aah, here is the init_pdata call I missed in patch 3. Sorry for the noise. > } > > /* > @@ -1640,6 +1646,7 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) > ppriv = SCHED_OP(new_ops, alloc_pdata, cpu); > if ( ppriv == NULL ) > return -ENOMEM; > + SCHED_OP(new_ops, init_pdata, ppriv, cpu); > vpriv = SCHED_OP(new_ops, alloc_vdata, idle, idle->domain->sched_priv); > if ( vpriv == NULL ) > { > diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h > index 825f1ad..70c08c6 100644 > --- a/xen/include/xen/sched-if.h > +++ b/xen/include/xen/sched-if.h > @@ -133,6 +133,7 @@ struct scheduler { > void *); > void (*free_pdata) (const struct scheduler *, void *, int); > void * (*alloc_pdata) (const struct scheduler *, int); > + void (*init_pdata) (const struct scheduler *, void *, int); > void (*free_domdata) (const struct scheduler *, void *); > void * (*alloc_domdata) (const struct scheduler *, struct domain *); Reviewed-by: Juergen Gross <jgross@suse.com>
On 18/03/16 19:04, Dario Faggioli wrote: > with the purpose of decoupling the allocation phase and > the initialization one, for per-pCPU data of the schedulers. > > This makes it possible to perform the initialization later > in the pCPU bringup/assignement process, when more information > (for instance, the host CPU topology) are available. This, > for now, is important only for Credit2, but it can well be > useful to other schedulers. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com> Just one note -- I found this patch harder to review than necessary, I think, because it implemented the callback but nobody was using it. I had to keep switching back and forth between the patches to find out what was going on. I personally would have folded patches 2 and 4 together. (Just to be clear, no action necessary.) > --- > Cc: George Dunlap <george.dunlap@eu.citrix.com> > Cc: Juergen Gross <jgross@suse.com> > --- > Changes from v1: > * in schedule_cpu_switch(), call to init_pdata() moved up, > close to the call to alloc_pdata() (for consistency with > other call sites) and prototype slightly changed. > --- > During v1 review, it was agreed to add ASSERTS() and comments > to clarify the use of schedule_cpu_switch(). This can't be > found here, but only because it has happened in another patch. > --- > xen/common/schedule.c | 7 +++++++ > xen/include/xen/sched-if.h | 1 + > 2 files changed, 8 insertions(+) > > diff --git a/xen/common/schedule.c b/xen/common/schedule.c > index e57b659..0627eb5 100644 > --- a/xen/common/schedule.c > +++ b/xen/common/schedule.c > @@ -1517,10 +1517,15 @@ static int cpu_schedule_callback( > struct notifier_block *nfb, unsigned long action, void *hcpu) > { > unsigned int cpu = (unsigned long)hcpu; > + struct scheduler *sched = per_cpu(scheduler, cpu); > + struct schedule_data *sd = &per_cpu(schedule_data, cpu); > int rc = 0; > > switch ( action ) > { > + case CPU_STARTING: > + SCHED_OP(sched, init_pdata, sd->sched_priv, cpu); > + break; > case CPU_UP_PREPARE: > rc = cpu_schedule_up(cpu); > break; > @@ -1597,6 +1602,7 @@ void __init scheduler_init(void) > if ( ops.alloc_pdata && > !(this_cpu(schedule_data).sched_priv = ops.alloc_pdata(&ops, 0)) ) > BUG(); > + SCHED_OP(&ops, init_pdata, this_cpu(schedule_data).sched_priv, 0); > } > > /* > @@ -1640,6 +1646,7 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) > ppriv = SCHED_OP(new_ops, alloc_pdata, cpu); > if ( ppriv == NULL ) > return -ENOMEM; > + SCHED_OP(new_ops, init_pdata, ppriv, cpu); > vpriv = SCHED_OP(new_ops, alloc_vdata, idle, idle->domain->sched_priv); > if ( vpriv == NULL ) > { > diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h > index 825f1ad..70c08c6 100644 > --- a/xen/include/xen/sched-if.h > +++ b/xen/include/xen/sched-if.h > @@ -133,6 +133,7 @@ struct scheduler { > void *); > void (*free_pdata) (const struct scheduler *, void *, int); > void * (*alloc_pdata) (const struct scheduler *, int); > + void (*init_pdata) (const struct scheduler *, void *, int); > void (*free_domdata) (const struct scheduler *, void *); > void * (*alloc_domdata) (const struct scheduler *, struct domain *); > >
diff --git a/xen/common/schedule.c b/xen/common/schedule.c index e57b659..0627eb5 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -1517,10 +1517,15 @@ static int cpu_schedule_callback( struct notifier_block *nfb, unsigned long action, void *hcpu) { unsigned int cpu = (unsigned long)hcpu; + struct scheduler *sched = per_cpu(scheduler, cpu); + struct schedule_data *sd = &per_cpu(schedule_data, cpu); int rc = 0; switch ( action ) { + case CPU_STARTING: + SCHED_OP(sched, init_pdata, sd->sched_priv, cpu); + break; case CPU_UP_PREPARE: rc = cpu_schedule_up(cpu); break; @@ -1597,6 +1602,7 @@ void __init scheduler_init(void) if ( ops.alloc_pdata && !(this_cpu(schedule_data).sched_priv = ops.alloc_pdata(&ops, 0)) ) BUG(); + SCHED_OP(&ops, init_pdata, this_cpu(schedule_data).sched_priv, 0); } /* @@ -1640,6 +1646,7 @@ int schedule_cpu_switch(unsigned int cpu, struct cpupool *c) ppriv = SCHED_OP(new_ops, alloc_pdata, cpu); if ( ppriv == NULL ) return -ENOMEM; + SCHED_OP(new_ops, init_pdata, ppriv, cpu); vpriv = SCHED_OP(new_ops, alloc_vdata, idle, idle->domain->sched_priv); if ( vpriv == NULL ) { diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h index 825f1ad..70c08c6 100644 --- a/xen/include/xen/sched-if.h +++ b/xen/include/xen/sched-if.h @@ -133,6 +133,7 @@ struct scheduler { void *); void (*free_pdata) (const struct scheduler *, void *, int); void * (*alloc_pdata) (const struct scheduler *, int); + void (*init_pdata) (const struct scheduler *, void *, int); void (*free_domdata) (const struct scheduler *, void *); void * (*alloc_domdata) (const struct scheduler *, struct domain *);
with the purpose of decoupling the allocation phase and the initialization one, for per-pCPU data of the schedulers. This makes it possible to perform the initialization later in the pCPU bringup/assignement process, when more information (for instance, the host CPU topology) are available. This, for now, is important only for Credit2, but it can well be useful to other schedulers. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> --- Cc: George Dunlap <george.dunlap@eu.citrix.com> Cc: Juergen Gross <jgross@suse.com> --- Changes from v1: * in schedule_cpu_switch(), call to init_pdata() moved up, close to the call to alloc_pdata() (for consistency with other call sites) and prototype slightly changed. --- During v1 review, it was agreed to add ASSERTS() and comments to clarify the use of schedule_cpu_switch(). This can't be found here, but only because it has happened in another patch. --- xen/common/schedule.c | 7 +++++++ xen/include/xen/sched-if.h | 1 + 2 files changed, 8 insertions(+)