Re: [Xen-devel] [PATCH v3 2/2] xen: credit1: avoid boosting vCPUs being "just" migrated

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Wed, 2016-02-24 at 10:43 +0000, George Dunlap wrote:
> On 12/02/16 16:29, Dario Faggioli wrote:
> >Â
> > @@ -1022,11 +1037,18 @@ csched_vcpu_wake(const struct scheduler
> > *ops, struct vcpu *vc)
> > ÂÂÂÂÂÂ* more CPU resource intensive VCPUs without impacting
> > overallÂ
> > ÂÂÂÂÂÂ* system fairness.
> > ÂÂÂÂÂÂ*
> > -ÂÂÂÂÂ* The one exception is for VCPUs of capped domains unpausing
> > -ÂÂÂÂÂ* after earning credits they had overspent. We don't boost
> > -ÂÂÂÂÂ* those.
> > +ÂÂÂÂÂ* There are two cases, when we don't want to boost:
> > +ÂÂÂÂÂ*ÂÂ- VCPUs that are waking up after a migration, rather than
> > +ÂÂÂÂÂ*ÂÂÂÂafter having block;
> > +ÂÂÂÂÂ*ÂÂ- VCPUs of capped domains unpausing after earning credits
> > +ÂÂÂÂÂ*ÂÂÂÂthey had overspent.
> > +ÂÂÂÂÂ*
> > +ÂÂÂÂÂ* Note that checking whether we are "only" migrating must be
> > +ÂÂÂÂÂ* done up front, as we do not want the clearing of the bit we
> > +ÂÂÂÂÂ* set in csched_cpu_pick() to be short-circuited away.
> > ÂÂÂÂÂÂ*/
> > -ÂÂÂÂif ( svc->pri == CSCHED_PRI_TS_UNDER &&
> > +ÂÂÂÂif ( !test_and_clear_bit(CSCHED_FLAG_VCPU_MIGRATING, &svc-
> > >flags)ÂÂ&&
> > +ÂÂÂÂÂÂÂÂÂsvc->pri == CSCHED_PRI_TS_UNDER &&
> > ÂÂÂÂÂÂÂÂÂÂ!test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
> 
> Sorry to be late reviewing this.
> 
No problem. Thanks for getting to it, actually, as I've got a few more
patches stacked on top of these outstanding series.

> So we always want to clear the 'migrating' flag, regardless of
> whether
> we do anything with boosting.ÂÂWould that logic be clearer if we
> cleared
> it as a separate step, storing the result in a local variable?ÂÂE.g.:
> 
> bool migrating;
> 
> ...
> 
> /* Always clear migrating flag if it's set */
> migrating = test_and_clear_bit(...)
> 
> if ( !migrating && ...) {
> }
> 
> Then we wouldn't need the last paragraph in the comment.
> 
Yes, I think I like this better.

> That said, this is v3, so if you'd rather just get this in as it is,
> then you can have my Acked-by as well.
> 
No, I'll resend... If I make (only) this change, can I resend directly
with your Acked-by?

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