diff mbox

xen: sched: get rid of static private schedulers' structures

Message ID 20160208131916.27675.143.stgit@Solace.station (mailing list archive)
State New, archived
Headers show

Commit Message

Dario Faggioli Feb. 8, 2016, 1:19 p.m. UTC
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(-)

Comments

Jan Beulich Feb. 9, 2016, 11:27 a.m. UTC | #1
>>> 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
Dario Faggioli Feb. 9, 2016, 12:25 p.m. UTC | #2
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
George Dunlap Feb. 23, 2016, 12:29 p.m. UTC | #3
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 mbox

Patch

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 *,