* [PATCH] xen: sched: get rid of static private schedulers' structures
@ 2016-02-08 13:19 Dario Faggioli
2016-02-09 11:27 ` Jan Beulich
2016-02-23 12:29 ` George Dunlap
0 siblings, 2 replies; 4+ messages in thread
From: Dario Faggioli @ 2016-02-08 13:19 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, George Dunlap
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(-)
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 *,
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xen: sched: get rid of static private schedulers' structures
2016-02-08 13:19 [PATCH] xen: sched: get rid of static private schedulers' structures Dario Faggioli
@ 2016-02-09 11:27 ` Jan Beulich
2016-02-09 12:25 ` Dario Faggioli
2016-02-23 12:29 ` George Dunlap
1 sibling, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2016-02-09 11:27 UTC (permalink / raw)
To: Dario Faggioli; +Cc: Juergen Gross, xen-devel, George Dunlap
>>> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xen: sched: get rid of static private schedulers' structures
2016-02-09 11:27 ` Jan Beulich
@ 2016-02-09 12:25 ` Dario Faggioli
0 siblings, 0 replies; 4+ messages in thread
From: Dario Faggioli @ 2016-02-09 12:25 UTC (permalink / raw)
To: Jan Beulich; +Cc: Juergen Gross, xen-devel, George Dunlap
[-- Attachment #1.1: Type: text/plain, Size: 1610 bytes --]
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
--
<<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 #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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xen: sched: get rid of static private schedulers' structures
2016-02-08 13:19 [PATCH] xen: sched: get rid of static private schedulers' structures Dario Faggioli
2016-02-09 11:27 ` Jan Beulich
@ 2016-02-23 12:29 ` George Dunlap
1 sibling, 0 replies; 4+ messages in thread
From: George Dunlap @ 2016-02-23 12:29 UTC (permalink / raw)
To: Dario Faggioli, xen-devel; +Cc: Juergen Gross
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 *,
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-23 12:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-08 13:19 [PATCH] xen: sched: get rid of static private schedulers' structures Dario Faggioli
2016-02-09 11:27 ` Jan Beulich
2016-02-09 12:25 ` Dario Faggioli
2016-02-23 12:29 ` George Dunlap
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.