diff mbox

[5/6] xen: RTDS: rearrange members of control structures

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

Commit Message

Dario Faggioli June 23, 2017, 10:55 a.m. UTC
Nothing changed in `pahole` output, in terms of holes
and padding, but some fields have been moved, to put
related members in same cache line.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: Meng Xu <mengxu@cis.upenn.edu>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
 xen/common/sched_rt.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

George Dunlap July 21, 2017, 5:06 p.m. UTC | #1
On 06/23/2017 11:55 AM, Dario Faggioli wrote:
> Nothing changed in `pahole` output, in terms of holes
> and padding, but some fields have been moved, to put
> related members in same cache line.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

Acked-by: George Dunlap <george.dunlap@citrix.com>

> ---
> Cc: Meng Xu <mengxu@cis.upenn.edu>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> ---
>  xen/common/sched_rt.c |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
> index 1b30014..39f6bee 100644
> --- a/xen/common/sched_rt.c
> +++ b/xen/common/sched_rt.c
> @@ -171,11 +171,14 @@ static void repl_timer_handler(void *data);
>  struct rt_private {
>      spinlock_t lock;            /* the global coarse-grained lock */
>      struct list_head sdom;      /* list of availalbe domains, used for dump */
> +
>      struct list_head runq;      /* ordered list of runnable vcpus */
>      struct list_head depletedq; /* unordered list of depleted vcpus */
> +
> +    struct timer *repl_timer;   /* replenishment timer */
>      struct list_head replq;     /* ordered list of vcpus that need replenishment */
> +
>      cpumask_t tickled;          /* cpus been tickled */
> -    struct timer *repl_timer;   /* replenishment timer */
>  };
>  
>  /*
> @@ -185,10 +188,6 @@ struct rt_vcpu {
>      struct list_head q_elem;     /* on the runq/depletedq list */
>      struct list_head replq_elem; /* on the replenishment events list */
>  
> -    /* Up-pointers */
> -    struct rt_dom *sdom;
> -    struct vcpu *vcpu;
> -
>      /* VCPU parameters, in nanoseconds */
>      s_time_t period;
>      s_time_t budget;
> @@ -198,6 +197,10 @@ struct rt_vcpu {
>      s_time_t last_start;         /* last start time */
>      s_time_t cur_deadline;       /* current deadline for EDF */
>  
> +    /* Up-pointers */
> +    struct rt_dom *sdom;
> +    struct vcpu *vcpu;
> +
>      unsigned flags;              /* mark __RTDS_scheduled, etc.. */
>  };
>  
>
Meng Xu July 21, 2017, 5:51 p.m. UTC | #2
On Fri, Jun 23, 2017 at 6:55 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
>
> Nothing changed in `pahole` output, in terms of holes
> and padding, but some fields have been moved, to put
> related members in same cache line.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> ---
> Cc: Meng Xu <mengxu@cis.upenn.edu>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> ---
>  xen/common/sched_rt.c |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
> index 1b30014..39f6bee 100644
> --- a/xen/common/sched_rt.c
> +++ b/xen/common/sched_rt.c
> @@ -171,11 +171,14 @@ static void repl_timer_handler(void *data);
>  struct rt_private {
>      spinlock_t lock;            /* the global coarse-grained lock */
>      struct list_head sdom;      /* list of availalbe domains, used for dump */
> +
>      struct list_head runq;      /* ordered list of runnable vcpus */
>      struct list_head depletedq; /* unordered list of depleted vcpus */
> +
> +    struct timer *repl_timer;   /* replenishment timer */
>      struct list_head replq;     /* ordered list of vcpus that need replenishment */
> +
>      cpumask_t tickled;          /* cpus been tickled */
> -    struct timer *repl_timer;   /* replenishment timer */
>  };
>
>  /*
> @@ -185,10 +188,6 @@ struct rt_vcpu {
>      struct list_head q_elem;     /* on the runq/depletedq list */
>      struct list_head replq_elem; /* on the replenishment events list */
>
> -    /* Up-pointers */
> -    struct rt_dom *sdom;
> -    struct vcpu *vcpu;
> -
>      /* VCPU parameters, in nanoseconds */
>      s_time_t period;
>      s_time_t budget;
> @@ -198,6 +197,10 @@ struct rt_vcpu {
>      s_time_t last_start;         /* last start time */
>      s_time_t cur_deadline;       /* current deadline for EDF */
>
> +    /* Up-pointers */
> +    struct rt_dom *sdom;
> +    struct vcpu *vcpu;
> +
>      unsigned flags;              /* mark __RTDS_scheduled, etc.. */
>  };
>

Reviewed-by: Meng Xu <mengxu@cis.upenn.edu>

BTW, Dario, I'm wondering if you used any tool to give hints about how
to arrange the fields in a structure or you just did it manually?

Thanks,

Meng

-----------
Meng Xu
PhD Candidate in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/
Dario Faggioli July 21, 2017, 7:51 p.m. UTC | #3
On Fri, 2017-07-21 at 13:51 -0400, Meng Xu wrote:
> On Fri, Jun 23, 2017 at 6:55 AM, Dario Faggioli
> <dario.faggioli@citrix.com> wrote:
> > 
> > Nothing changed in `pahole` output, in terms of holes
> > and padding, but some fields have been moved, to put
> > related members in same cache line.
> > 
> > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> > ---
> > Cc: Meng Xu <mengxu@cis.upenn.edu>
> > Cc: George Dunlap <george.dunlap@eu.citrix.com>
> > ---
> >  xen/common/sched_rt.c |   13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
> > index 1b30014..39f6bee 100644
> > --- a/xen/common/sched_rt.c
> > +++ b/xen/common/sched_rt.c
> > @@ -171,11 +171,14 @@ static void repl_timer_handler(void *data);
> >  struct rt_private {
> >      spinlock_t lock;            /* the global coarse-grained lock
> > */
> >      struct list_head sdom;      /* list of availalbe domains, used
> > for dump */
> > +
> >      struct list_head runq;      /* ordered list of runnable vcpus
> > */
> >      struct list_head depletedq; /* unordered list of depleted
> > vcpus */
> > +
> > +    struct timer *repl_timer;   /* replenishment timer */
> >      struct list_head replq;     /* ordered list of vcpus that need
> > replenishment */
> > +
> >      cpumask_t tickled;          /* cpus been tickled */
> > -    struct timer *repl_timer;   /* replenishment timer */
> >  };
> > 
> >  /*
> > @@ -185,10 +188,6 @@ struct rt_vcpu {
> >      struct list_head q_elem;     /* on the runq/depletedq list */
> >      struct list_head replq_elem; /* on the replenishment events
> > list */
> > 
> > -    /* Up-pointers */
> > -    struct rt_dom *sdom;
> > -    struct vcpu *vcpu;
> > -
> >      /* VCPU parameters, in nanoseconds */
> >      s_time_t period;
> >      s_time_t budget;
> > @@ -198,6 +197,10 @@ struct rt_vcpu {
> >      s_time_t last_start;         /* last start time */
> >      s_time_t cur_deadline;       /* current deadline for EDF */
> > 
> > +    /* Up-pointers */
> > +    struct rt_dom *sdom;
> > +    struct vcpu *vcpu;
> > +
> >      unsigned flags;              /* mark __RTDS_scheduled, etc..
> > */
> >  };
> > 
> 
> Reviewed-by: Meng Xu <mengxu@cis.upenn.edu>
> 
> BTW, Dario, I'm wondering if you used any tool to give hints about
> how
> to arrange the fields in a structure or you just did it manually?
> 
I used pahole for figuring out the cache layout. But just that. So,
basically, I --manually-- tried to move the fields around, and check
the result with pahole (and then did it again, and again. :-D).

TBH, the improvement for RTDS is probably not even noticeable, as we
access almost all the fields anyway. But it still makes sense, IMO.

Thanks for the review,
Dario
diff mbox

Patch

diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 1b30014..39f6bee 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -171,11 +171,14 @@  static void repl_timer_handler(void *data);
 struct rt_private {
     spinlock_t lock;            /* the global coarse-grained lock */
     struct list_head sdom;      /* list of availalbe domains, used for dump */
+
     struct list_head runq;      /* ordered list of runnable vcpus */
     struct list_head depletedq; /* unordered list of depleted vcpus */
+
+    struct timer *repl_timer;   /* replenishment timer */
     struct list_head replq;     /* ordered list of vcpus that need replenishment */
+
     cpumask_t tickled;          /* cpus been tickled */
-    struct timer *repl_timer;   /* replenishment timer */
 };
 
 /*
@@ -185,10 +188,6 @@  struct rt_vcpu {
     struct list_head q_elem;     /* on the runq/depletedq list */
     struct list_head replq_elem; /* on the replenishment events list */
 
-    /* Up-pointers */
-    struct rt_dom *sdom;
-    struct vcpu *vcpu;
-
     /* VCPU parameters, in nanoseconds */
     s_time_t period;
     s_time_t budget;
@@ -198,6 +197,10 @@  struct rt_vcpu {
     s_time_t last_start;         /* last start time */
     s_time_t cur_deadline;       /* current deadline for EDF */
 
+    /* Up-pointers */
+    struct rt_dom *sdom;
+    struct vcpu *vcpu;
+
     unsigned flags;              /* mark __RTDS_scheduled, etc.. */
 };