[PATCH v3] credit: generalize __vcpu_has_soft_affinity()

[PATCH v3] credit: generalize __vcpu_has_soft_affinity()

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2525 bytes --]

As pointed out in the discussion of the patch at
http://lists.xenproject.org/archives/html/xen-devel/2015-02/msg03256.html 
generalizing the conditions here means code elsewhere doesn't need to
take into consideration internals of how load balancing in the credit
scheduler works.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
---
v3: Swap soft and hard affinities in the 2nd of the 3 checks. Alter
    comment as requested by George (which made us notice the need for
    the code change, therefore retaining his review tag).
v2: Use VCPU2ONLINE(vc) (or really an open coded variant thereof)
    instead of cpu_online_map (suggested by Dario).

--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -279,24 +279,23 @@ __runq_remove(struct csched_vcpu *svc)
 
 /*
  * Hard affinity balancing is always necessary and must never be skipped.
- * OTOH, if the vcpu's soft affinity is full (it spans all the possible
- * pcpus) we can safely avoid dealing with it entirely.
+ * But soft affinity need only be considered when it has a functionally
+ * different effect than other constraints (such as hard affinity, cpus
+ * online, or cpupools).
  *
- * A vcpu's soft affinity is also deemed meaningless in case it has empty
- * intersection with mask, to cover the cases where using the soft affinity
- * mask seems legit, but would instead led to trying to schedule the vcpu
- * on _no_ pcpu! Typical use cases are for mask to be equal to the vcpu's
- * hard affinity, or to the && of hard affinity and the set of online cpus
- * in the domain's cpupool.
+ * Soft affinity only needs to be considered if:
+ * * The cpus in the cpupool are not a subset of soft affinity
+ * * The hard affinity is not a subset of soft affinity
+ * * There is an overlap between the soft affinity and the mask which is
+ *   currently being considered.
  */
 static inline int __vcpu_has_soft_affinity(const struct vcpu *vc,
                                            const cpumask_t *mask)
 {
-    if ( cpumask_full(vc->cpu_soft_affinity)
-         || !cpumask_intersects(vc->cpu_soft_affinity, mask) )
-        return 0;
-
-    return 1;
+    return !cpumask_subset(cpupool_online_cpumask(vc->domain->cpupool),
+                           vc->cpu_soft_affinity) &&
+           !cpumask_subset(vc->cpu_hard_affinity, vc->cpu_soft_affinity) &&
+           cpumask_intersects(vc->cpu_soft_affinity, mask);
 }
 
 /*




[-- Attachment #2: credit-has-soft-affinity.patch --]
[-- Type: text/plain, Size: 2567 bytes --]

credit: generalize __vcpu_has_soft_affinity()

As pointed out in the discussion of the patch at
http://lists.xenproject.org/archives/html/xen-devel/2015-02/msg03256.html
generalizing the conditions here means code elsewhere doesn't need to
take into consideration internals of how load balancing in the credit
scheduler works.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
---
v3: Swap soft and hard affinities in the 2nd of the 3 checks. Alter
    comment as requested by George (which made us notice the need for
    the code change, therefore retaining his review tag).
v2: Use VCPU2ONLINE(vc) (or really an open coded variant thereof)
    instead of cpu_online_map (suggested by Dario).

--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -279,24 +279,23 @@ __runq_remove(struct csched_vcpu *svc)
 
 /*
  * Hard affinity balancing is always necessary and must never be skipped.
- * OTOH, if the vcpu's soft affinity is full (it spans all the possible
- * pcpus) we can safely avoid dealing with it entirely.
+ * But soft affinity need only be considered when it has a functionally
+ * different effect than other constraints (such as hard affinity, cpus
+ * online, or cpupools).
  *
- * A vcpu's soft affinity is also deemed meaningless in case it has empty
- * intersection with mask, to cover the cases where using the soft affinity
- * mask seems legit, but would instead led to trying to schedule the vcpu
- * on _no_ pcpu! Typical use cases are for mask to be equal to the vcpu's
- * hard affinity, or to the && of hard affinity and the set of online cpus
- * in the domain's cpupool.
+ * Soft affinity only needs to be considered if:
+ * * The cpus in the cpupool are not a subset of soft affinity
+ * * The hard affinity is not a subset of soft affinity
+ * * There is an overlap between the soft affinity and the mask which is
+ *   currently being considered.
  */
 static inline int __vcpu_has_soft_affinity(const struct vcpu *vc,
                                            const cpumask_t *mask)
 {
-    if ( cpumask_full(vc->cpu_soft_affinity)
-         || !cpumask_intersects(vc->cpu_soft_affinity, mask) )
-        return 0;
-
-    return 1;
+    return !cpumask_subset(cpupool_online_cpumask(vc->domain->cpupool),
+                           vc->cpu_soft_affinity) &&
+           !cpumask_subset(vc->cpu_hard_affinity, vc->cpu_soft_affinity) &&
+           cpumask_intersects(vc->cpu_soft_affinity, mask);
 }
 
 /*

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

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

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

[Dario Faggioli]

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

On Fri, 2015-03-06 at 13:29 +0000, Jan Beulich wrote:
> As pointed out in the discussion of the patch at
> http://lists.xenproject.org/archives/html/xen-devel/2015-02/msg03256.html 
> generalizing the conditions here means code elsewhere doesn't need to
> take into consideration internals of how load balancing in the credit
> scheduler works.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> v3: Swap soft and hard affinities in the 2nd of the 3 checks. Alter
>     comment as requested by George (which made us notice the need for
>     the code change, therefore retaining his review tag).
> v2: Use VCPU2ONLINE(vc) (or really an open coded variant thereof)
>     instead of cpu_online_map (suggested by Dario).
> 
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

This time, hopefully, without having overlooked anything! :-)

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