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

[George Dunlap <george.dunlap@citrix.com>]

On 24/02/16 11:12, Dario Faggioli wrote:
> 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?

Well I won't know what it looks like until I see it, will I? :-)  I
prioritize recently-reviewed series, so if you resend I should be able
to give an Acked-by the same day.

 -George