| Message ID | 20160208131916.27675.143.stgit@Solace.station (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 08.02.16 at 14:19, <dario.faggioli@citrix.com> wrote: > @@ -1995,13 +1996,11 @@ static void csched_tick_resume(const struct scheduler *ops, unsigned int cpu) > - now % MICROSECS(prv->tick_period_us) ); > } > > -static struct csched_private _csched_priv; > - > static const struct scheduler sched_credit_def = { > .name = "SMP Credit Scheduler", > .opt_name = "credit", > .sched_id = XEN_SCHEDULER_CREDIT, > - .sched_data = &_csched_priv, > + .sched_data = NULL, You're maintainer of this code, so you know whether you really want these NULL initializers, but the look pretty pointless to me. Jan
On Tue, 2016-02-09 at 04:27 -0700, Jan Beulich wrote: > > > > On 08.02.16 at 14:19, <dario.faggioli@citrix.com> wrote: > > @@ -1995,13 +1996,11 @@ static void csched_tick_resume(const struct > > scheduler *ops, unsigned int cpu) > > - now % MICROSECS(prv->tick_period_us) ); > > } > > > > -static struct csched_private _csched_priv; > > - > > static const struct scheduler sched_credit_def = { > > .name = "SMP Credit Scheduler", > > .opt_name = "credit", > > .sched_id = XEN_SCHEDULER_CREDIT, > > - .sched_data = &_csched_priv, > > + .sched_data = NULL, > > You're maintainer of this code, so you know whether you really > want these NULL initializers, but the look pretty pointless to me. > Yes, I've also been unsure about what to do here. The reason why I put them there is that, when looking at the code, it is important to be able to quickly get the idea of where sched_data points, at any given time. Therefore, being a little bit more verbose than necessary would be, in this case, actually helpful. Anyway, this is not a super strong opinion, so I can be talked into getting rid of them, if others think we better don't have them too. Thanks and Regards, Dario
On 08/02/16 13:19, Dario Faggioli wrote: > In fact, they look rather useless: they are never > referenced neither directly, nor via the sched_data > pointer, as a dynamic copy that overrides them is > allocated as the very first step of a scheduler's > initialization. > > While there, take the chance to also reset the sched_data > pointer to NULL, upon scheduler de-initialization. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Acked-by: George Dunlap <george.dunlap@citrix.com> Sorry for the delay > --- > Cc: George Dunlap <george.dunlap@citrix.com> > Cc: Juergen Gross <jgross@suse.com> > --- > xen/common/sched_arinc653.c | 3 ++- > xen/common/sched_credit.c | 7 +++---- > xen/common/sched_credit2.c | 8 +++----- > xen/common/sched_rt.c | 7 +++---- > xen/include/xen/sched-if.h | 2 +- > 5 files changed, 12 insertions(+), 15 deletions(-) > > diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c > index 0606988..8a11a2f 100644 > --- a/xen/common/sched_arinc653.c > +++ b/xen/common/sched_arinc653.c > @@ -367,9 +367,10 @@ a653sched_init(struct scheduler *ops) > * @param ops Pointer to this instance of the scheduler structure > */ > static void > -a653sched_deinit(const struct scheduler *ops) > +a653sched_deinit(struct scheduler *ops) > { > xfree(SCHED_PRIV(ops)); > + ops->sched_data = NULL; > } > > /** > diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c > index 671bbee..25a4773 100644 > --- a/xen/common/sched_credit.c > +++ b/xen/common/sched_credit.c > @@ -1959,13 +1959,14 @@ csched_init(struct scheduler *ops) > } > > static void > -csched_deinit(const struct scheduler *ops) > +csched_deinit(struct scheduler *ops) > { > struct csched_private *prv; > > prv = CSCHED_PRIV(ops); > if ( prv != NULL ) > { > + ops->sched_data = NULL; > free_cpumask_var(prv->cpus); > free_cpumask_var(prv->idlers); > xfree(prv); > @@ -1995,13 +1996,11 @@ static void csched_tick_resume(const struct scheduler *ops, unsigned int cpu) > - now % MICROSECS(prv->tick_period_us) ); > } > > -static struct csched_private _csched_priv; > - > static const struct scheduler sched_credit_def = { > .name = "SMP Credit Scheduler", > .opt_name = "credit", > .sched_id = XEN_SCHEDULER_CREDIT, > - .sched_data = &_csched_priv, > + .sched_data = NULL, > > .init_domain = csched_dom_init, > .destroy_domain = csched_dom_destroy, > diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c > index 78220a7..68a8f1c 100644 > --- a/xen/common/sched_credit2.c > +++ b/xen/common/sched_credit2.c > @@ -2183,22 +2183,20 @@ csched2_init(struct scheduler *ops) > } > > static void > -csched2_deinit(const struct scheduler *ops) > +csched2_deinit(struct scheduler *ops) > { > struct csched2_private *prv; > > prv = CSCHED2_PRIV(ops); > + ops->sched_data = NULL; > xfree(prv); > } > > - > -static struct csched2_private _csched2_priv; > - > static const struct scheduler sched_credit2_def = { > .name = "SMP Credit Scheduler rev2", > .opt_name = "credit2", > .sched_id = XEN_SCHEDULER_CREDIT2, > - .sched_data = &_csched2_priv, > + .sched_data = NULL, > > .init_domain = csched2_dom_init, > .destroy_domain = csched2_dom_destroy, > diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c > index 2e5430f..d82d69c 100644 > --- a/xen/common/sched_rt.c > +++ b/xen/common/sched_rt.c > @@ -462,7 +462,7 @@ rt_init(struct scheduler *ops) > } > > static void > -rt_deinit(const struct scheduler *ops) > +rt_deinit(struct scheduler *ops) > { > struct rt_private *prv = rt_priv(ops); > > @@ -473,6 +473,7 @@ rt_deinit(const struct scheduler *ops) > xfree(_cpumask_scratch); > _cpumask_scratch = NULL; > } > + ops->sched_data = NULL; > xfree(prv); > } > > @@ -1168,13 +1169,11 @@ rt_dom_cntl( > return rc; > } > > -static struct rt_private _rt_priv; > - > static const struct scheduler sched_rtds_def = { > .name = "SMP RTDS Scheduler", > .opt_name = "rtds", > .sched_id = XEN_SCHEDULER_RTDS, > - .sched_data = &_rt_priv, > + .sched_data = NULL, > > .dump_cpu_state = rt_dump_pcpu, > .dump_settings = rt_dump, > diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h > index 66dc9c8..825f1ad 100644 > --- a/xen/include/xen/sched-if.h > +++ b/xen/include/xen/sched-if.h > @@ -126,7 +126,7 @@ struct scheduler { > int (*global_init) (void); > > int (*init) (struct scheduler *); > - void (*deinit) (const struct scheduler *); > + void (*deinit) (struct scheduler *); > > void (*free_vdata) (const struct scheduler *, void *); > void * (*alloc_vdata) (const struct scheduler *, struct vcpu *, >
diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c index 0606988..8a11a2f 100644 --- a/xen/common/sched_arinc653.c +++ b/xen/common/sched_arinc653.c @@ -367,9 +367,10 @@ a653sched_init(struct scheduler *ops) * @param ops Pointer to this instance of the scheduler structure */ static void -a653sched_deinit(const struct scheduler *ops) +a653sched_deinit(struct scheduler *ops) { xfree(SCHED_PRIV(ops)); + ops->sched_data = NULL; } /** diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index 671bbee..25a4773 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -1959,13 +1959,14 @@ csched_init(struct scheduler *ops) } static void -csched_deinit(const struct scheduler *ops) +csched_deinit(struct scheduler *ops) { struct csched_private *prv; prv = CSCHED_PRIV(ops); if ( prv != NULL ) { + ops->sched_data = NULL; free_cpumask_var(prv->cpus); free_cpumask_var(prv->idlers); xfree(prv); @@ -1995,13 +1996,11 @@ static void csched_tick_resume(const struct scheduler *ops, unsigned int cpu) - now % MICROSECS(prv->tick_period_us) ); } -static struct csched_private _csched_priv; - static const struct scheduler sched_credit_def = { .name = "SMP Credit Scheduler", .opt_name = "credit", .sched_id = XEN_SCHEDULER_CREDIT, - .sched_data = &_csched_priv, + .sched_data = NULL, .init_domain = csched_dom_init, .destroy_domain = csched_dom_destroy, diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c index 78220a7..68a8f1c 100644 --- a/xen/common/sched_credit2.c +++ b/xen/common/sched_credit2.c @@ -2183,22 +2183,20 @@ csched2_init(struct scheduler *ops) } static void -csched2_deinit(const struct scheduler *ops) +csched2_deinit(struct scheduler *ops) { struct csched2_private *prv; prv = CSCHED2_PRIV(ops); + ops->sched_data = NULL; xfree(prv); } - -static struct csched2_private _csched2_priv; - static const struct scheduler sched_credit2_def = { .name = "SMP Credit Scheduler rev2", .opt_name = "credit2", .sched_id = XEN_SCHEDULER_CREDIT2, - .sched_data = &_csched2_priv, + .sched_data = NULL, .init_domain = csched2_dom_init, .destroy_domain = csched2_dom_destroy, diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c index 2e5430f..d82d69c 100644 --- a/xen/common/sched_rt.c +++ b/xen/common/sched_rt.c @@ -462,7 +462,7 @@ rt_init(struct scheduler *ops) } static void -rt_deinit(const struct scheduler *ops) +rt_deinit(struct scheduler *ops) { struct rt_private *prv = rt_priv(ops); @@ -473,6 +473,7 @@ rt_deinit(const struct scheduler *ops) xfree(_cpumask_scratch); _cpumask_scratch = NULL; } + ops->sched_data = NULL; xfree(prv); } @@ -1168,13 +1169,11 @@ rt_dom_cntl( return rc; } -static struct rt_private _rt_priv; - static const struct scheduler sched_rtds_def = { .name = "SMP RTDS Scheduler", .opt_name = "rtds", .sched_id = XEN_SCHEDULER_RTDS, - .sched_data = &_rt_priv, + .sched_data = NULL, .dump_cpu_state = rt_dump_pcpu, .dump_settings = rt_dump, diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h index 66dc9c8..825f1ad 100644 --- a/xen/include/xen/sched-if.h +++ b/xen/include/xen/sched-if.h @@ -126,7 +126,7 @@ struct scheduler { int (*global_init) (void); int (*init) (struct scheduler *); - void (*deinit) (const struct scheduler *); + void (*deinit) (struct scheduler *); void (*free_vdata) (const struct scheduler *, void *); void * (*alloc_vdata) (const struct scheduler *, struct vcpu *,
In fact, they look rather useless: they are never referenced neither directly, nor via the sched_data pointer, as a dynamic copy that overrides them is allocated as the very first step of a scheduler's initialization. While there, take the chance to also reset the sched_data pointer to NULL, upon scheduler de-initialization. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> --- Cc: George Dunlap <george.dunlap@citrix.com> Cc: Juergen Gross <jgross@suse.com> --- xen/common/sched_arinc653.c | 3 ++- xen/common/sched_credit.c | 7 +++---- xen/common/sched_credit2.c | 8 +++----- xen/common/sched_rt.c | 7 +++---- xen/include/xen/sched-if.h | 2 +- 5 files changed, 12 insertions(+), 15 deletions(-)