Re: [Xen-devel] [PATCH 5/6] xen: RTDS: rearrange members of control structures

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > Cc: Meng Xu <mengxu@xxxxxxxxxxxxx>
> > Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxxxxx>
> 
> 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
-- 
<<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: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel