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

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2558 bytes --]

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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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