Re: [Xen-devel] [PATCH] sched_credit: Remove cpu argument to __runq_insert()

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Fri, 2015-10-30 at 10:25 -0600, Jan Beulich wrote:
> > > > On 30.10.15 at 16:09, <write.harmandeep@xxxxxxxxx> wrote:

> > --- a/xen/common/sched_credit.c
> > +++ b/xen/common/sched_credit.c
> > @@ -252,13 +252,12 @@ __runq_elem(struct list_head *elem)
> >  }
> >  
> >  static inline void
> > -__runq_insert(unsigned int cpu, struct csched_vcpu *svc)
> > +__runq_insert(struct csched_vcpu *svc)
> >  {
> > -    const struct list_head * const runq = RUNQ(cpu);
> > +    const struct list_head * const runq = RUNQ(svc->vcpu
> > ->processor);
> 
> ... this being an inline function the change will likely make the
> compiler produce worse code, if only ...
> 
> >      struct list_head *iter;
> >  
> >      BUG_ON( __vcpu_on_runq(svc) );
> > -    BUG_ON( cpu != svc->vcpu->processor );
> 
> ... this was an ASSERT() instead of a BUG_ON() (which it looks like
> it should be).
> 
Mmm... I'm sorry, but I'm not getting what you are actually suggesting.

Are you saying that we shouldn't make the change at all? Or that we
should make the change and also turn the BUG_ON() (the one that is left
in place) into an ASSERT()? Or that we should not mark the function as
'inline'?

Harman, perhaps, can you check the assembly code produced by the
compiler before and after your patch and report here what the
differences are?

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: signature.asc
Description: This is a digitally signed message part

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