Re: [PATCH v2] credit: generalize __vcpu_has_soft_affinity()

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

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

On Fri, 2015-03-06 at 12:00 +0000, George Dunlap wrote:
> >> On 03/06/2015 10:16 AM, Jan Beulich wrote:

> >>> Hmm, not sure. And I keep being confused whether soft means
> >>> "allow" and hard means "prefer" or the other way around. 
> >>>
> >> "hard" means allow (or not allow)
> >> "soft" means prefer
> > 
> > while I read your reply to mean the opposite.
> 
> No, Dario and I mean the same thing: "Hard affinity" means "May only run
> on these cpus and no others".  "Soft affinity" means "I prefer you to
> run here if you can, but if it's too busy, go ahead and run it somewhere
> else".
> 
Yep.

> Consider the following example
> 
> Soft affinity: 00001100
> Hard affinity: 00111100
> 
> In this case, the scheduler should *prefer* to run it on cpus 4 or 5
> (which is in both the soft and hard affinities), but *may* run it on
> cpus 2-3 if it thinks it's necessary; so there is an "effective soft
> affinity" -- even though soft affinity is a subset of hard affinity.
> 
Exactly.

So, in this case:

 cpumask_subset(vc->cpu_soft_affinity, vc->cpu_hard_affinity) = 1

And, as far as this check only is concerned, __vcpu_has_soft_affinity()
should return 'true'.

> Now consider the reverse:
> 
> Soft affinity: 00111100
> Hard affinity: 00001100
> 
> In this case, the scheduler *must* run it on either cpus 4 or 5; but
> there is no preference between the two, since both are in the soft
> affinity set.  So this is effectively the same as not having any soft
> affinity.  And hard affinity is a subset of soft affinity.
> 
Correct again. So, here:

 cpumask_subset(vc->cpu_soft_affinity, vc->cpu_hard_affinity) = 0

And __vcpu_has_soft_affinity() should (modulo the other parts) return
'false'.

Which means that, yes, this part of the condition must loose the '!', or
have the arguments of the call to cpumask_subset() switched. I
personally prefer the former:

    return !cpumask_subset(cpupool_online_cpumask(vc->domain->cpupool),
                           vc->cpu_soft_affinity) &&
           cpumask_subset(vc->cpu_soft_affinity, vc->cpu_hard_affinity) &&
           cpumask_intersects(vc->cpu_soft_affinity, mask);

At which point it may make sense to turn everything into 'affermative'
logic. It at least look easier to read here:

    return cpumask_subset(vc->cpu_soft_affinity,
                          cpupool_online_cpumask(vc->domain->cpupool) &&
           cpumask_subset(vc->cpu_soft_affinity, vc->cpu_hard_affinity) &&
           cpumask_intersects(vc->cpu_soft_affinity, mask);


IAC, sorry for overlooking this, I was focusing more on the very first
item of the conditional expression, and managed to miss this. :-(

Regards,
Dario

[-- 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