[PATCH 0/4] xen: sched: optimize exclusive pinning and soft-affinity checking

[PATCH 0/4] xen: sched: optimize exclusive pinning and soft-affinity checking

[Dario Faggioli]

Hello,

This series is a rework of a patch that was, originally, sent as part of a
series (which went in already):

 https://lists.xen.org/archives/html/xen-devel/2017-07/msg02167.html

As it can be seen in the message above, George suggested doing things a little
bit differently, and I agreed. however, that:
- require a bit more of rework than expected, but here we are;
- opened the possibility for even more optimization. :-)

Basically, the effect of the series is that:
1) when a vCPU is exclusively pinned to a pCPU, a lot of checks, while making
   scheduling decisions in Credit1 and Credit2, are skipped (patch 2);
2) the check to see whether or not a vCPU has a soft-affinity that actually
   matters for the scheduling of the vCPU itself, and should be considered is
   optimized and made faster (patch 4).

So, the important bits are in patches 2 and 4, but both patches 1 and 3 are
necessary to make the other twos possible.

Regards,
Dario
---
Dario Faggioli (4):
      xen: sched: introduce 'adjust_affinity' hook.
      xen: sched: optimize exclusive pinning case (Credit1 & 2)
      xen: sched: improve checking soft-affinity
      xen: sched: simplify (and speedup) checking soft-affinity

 xen/arch/x86/dom0_build.c    |    4 +
 xen/common/sched_credit.c    |  114 +++++++++++++++++++++++++++---------------
 xen/common/sched_credit2.c   |   50 ++++++++++++++++--
 xen/common/sched_null.c      |    8 +--
 xen/common/schedule.c        |   67 +++++++++++++++++++------
 xen/include/xen/perfc_defn.h |    1 
 xen/include/xen/sched-if.h   |   16 +++---
 xen/include/xen/sched.h      |    5 ++
 8 files changed, 188 insertions(+), 77 deletions(-)
--
<<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)

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

[PATCH 1/4] xen: sched: introduce 'adjust_affinity' hook.

[Dario Faggioli]

For now, just as a way to give a scheduler an "heads up",
about the fact that the affinity changed.

This enables some optimizations, such as pre-computing
and storing (e.g., in flags) facts like a vcpu being
exclusively pinned to a pcpu, or having or not a
soft affinity. I.e., conditions that, despite the fact
that they rarely change, are right now checked very
frequently, even in hot paths.

Note also that this, in future, may turn out as a useful
mean for, e.g., having the schedulers vet, ack or nack
the changes themselves.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Anshul Makkar <anshulmakkar@gmail.com>
---
 xen/arch/x86/dom0_build.c  |    4 +--
 xen/common/schedule.c      |   62 +++++++++++++++++++++++++++++++++-----------
 xen/include/xen/sched-if.h |    3 ++
 xen/include/xen/sched.h    |    3 ++
 4 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index f616b99..94e4b7f 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -125,8 +125,8 @@ struct vcpu *__init dom0_setup_vcpu(struct domain *d,
     if ( v )
     {
         if ( !d->is_pinned && !dom0_affinity_relaxed )
-            cpumask_copy(v->cpu_hard_affinity, &dom0_cpus);
-        cpumask_copy(v->cpu_soft_affinity, &dom0_cpus);
+            sched_set_affinity(v, &dom0_cpus, NULL);
+        sched_set_affinity(v, NULL, &dom0_cpus);
     }
 
     return v;
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 8827921..a8b82fd 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -256,6 +256,9 @@ static void sched_spin_unlock_double(spinlock_t *lock1, spinlock_t *lock2,
 int sched_init_vcpu(struct vcpu *v, unsigned int processor) 
 {
     struct domain *d = v->domain;
+    cpumask_t allcpus;
+
+    cpumask_setall(&allcpus);
 
     /*
      * Initialize processor and affinity settings. The idler, and potentially
@@ -263,11 +266,9 @@ int sched_init_vcpu(struct vcpu *v, unsigned int processor)
      */
     v->processor = processor;
     if ( is_idle_domain(d) || d->is_pinned )
-        cpumask_copy(v->cpu_hard_affinity, cpumask_of(processor));
+        sched_set_affinity(v, cpumask_of(processor), &allcpus);
     else
-        cpumask_setall(v->cpu_hard_affinity);
-
-    cpumask_setall(v->cpu_soft_affinity);
+        sched_set_affinity(v, &allcpus, &allcpus);
 
     /* Initialise the per-vcpu timers. */
     init_timer(&v->periodic_timer, vcpu_periodic_timer_fn,
@@ -359,6 +360,7 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
     for_each_vcpu ( d, v )
     {
         spinlock_t *lock;
+        cpumask_t allcpus;
 
         vcpudata = v->sched_priv;
 
@@ -366,10 +368,12 @@ int sched_move_domain(struct domain *d, struct cpupool *c)
         migrate_timer(&v->singleshot_timer, new_p);
         migrate_timer(&v->poll_timer, new_p);
 
-        cpumask_setall(v->cpu_hard_affinity);
-        cpumask_setall(v->cpu_soft_affinity);
+        cpumask_setall(&allcpus);
 
         lock = vcpu_schedule_lock_irq(v);
+
+        sched_set_affinity(v, &allcpus, &allcpus);
+
         v->processor = new_p;
         /*
          * With v->processor modified we must not
@@ -680,7 +684,7 @@ void restore_vcpu_affinity(struct domain *d)
 
         if ( v->affinity_broken )
         {
-            cpumask_copy(v->cpu_hard_affinity, v->cpu_hard_affinity_saved);
+            sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
             v->affinity_broken = 0;
 
         }
@@ -744,6 +748,8 @@ int cpu_disable_scheduler(unsigned int cpu)
             if ( cpumask_empty(&online_affinity) &&
                  cpumask_test_cpu(cpu, v->cpu_hard_affinity) )
             {
+                cpumask_t allcpus;
+
                 if ( v->affinity_broken )
                 {
                     /* The vcpu is temporarily pinned, can't move it. */
@@ -761,7 +767,8 @@ int cpu_disable_scheduler(unsigned int cpu)
                 else
                     printk(XENLOG_DEBUG "Breaking affinity for %pv\n", v);
 
-                cpumask_setall(v->cpu_hard_affinity);
+                cpumask_setall(&allcpus);
+                sched_set_affinity(v, &allcpus, NULL);
             }
 
             if ( v->processor != cpu )
@@ -831,8 +838,26 @@ int cpu_disable_scheduler(unsigned int cpu)
     return ret;
 }
 
+/*
+ * In general, this must be called with the scheduler lock held, because the
+ * adjust_affinity hook may want to modify the vCPU state. However, when the
+ * vCPU is being initialized (either for dom0 or domU) there is no risk of
+ * races, and it's fine to not take the look (we're talking about
+ * dom0_setup_vcpu() an sched_init_vcpu()).
+ */
+void sched_set_affinity(
+    struct vcpu *v, const cpumask_t *hard, const cpumask_t *soft)
+{
+    SCHED_OP(dom_scheduler(v->domain), adjust_affinity, v, hard, soft);
+
+    if ( hard )
+        cpumask_copy(v->cpu_hard_affinity, hard);
+    if ( soft )
+        cpumask_copy(v->cpu_hard_affinity, soft);
+}
+
 static int vcpu_set_affinity(
-    struct vcpu *v, const cpumask_t *affinity, cpumask_t *which)
+    struct vcpu *v, const cpumask_t *affinity, const cpumask_t *which)
 {
     spinlock_t *lock;
     int ret = 0;
@@ -843,12 +868,19 @@ static int vcpu_set_affinity(
         ret = -EBUSY;
     else
     {
-        cpumask_copy(which, affinity);
-
         /*
-         * Always ask the scheduler to re-evaluate placement
-         * when changing the affinity.
+         * Tell the scheduler we changes something about affinity,
+         * and ask to re-evaluate vcpu placement.
          */
+        if ( which == v->cpu_hard_affinity )
+        {
+            sched_set_affinity(v, affinity, NULL);
+        }
+        else
+        {
+            ASSERT(which == v->cpu_soft_affinity);
+            sched_set_affinity(v, NULL, affinity);
+        }
         set_bit(_VPF_migrating, &v->pause_flags);
     }
 
@@ -1086,7 +1118,7 @@ int vcpu_pin_override(struct vcpu *v, int cpu)
     {
         if ( v->affinity_broken )
         {
-            cpumask_copy(v->cpu_hard_affinity, v->cpu_hard_affinity_saved);
+            sched_set_affinity(v, v->cpu_hard_affinity_saved, NULL);
             v->affinity_broken = 0;
             set_bit(_VPF_migrating, &v->pause_flags);
             ret = 0;
@@ -1100,7 +1132,7 @@ int vcpu_pin_override(struct vcpu *v, int cpu)
         {
             cpumask_copy(v->cpu_hard_affinity_saved, v->cpu_hard_affinity);
             v->affinity_broken = 1;
-            cpumask_copy(v->cpu_hard_affinity, cpumask_of(cpu));
+            sched_set_affinity(v, cpumask_of(cpu), NULL);
             set_bit(_VPF_migrating, &v->pause_flags);
             ret = 0;
         }
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index c4a4935..982c780 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -172,6 +172,9 @@ struct scheduler {
                                     unsigned int);
     int          (*adjust)         (const struct scheduler *, struct domain *,
                                     struct xen_domctl_scheduler_op *);
+    void         (*adjust_affinity)(const struct scheduler *, struct vcpu *,
+                                    const struct cpumask *,
+                                    const struct cpumask *);
     int          (*adjust_global)  (const struct scheduler *,
                                     struct xen_sysctl_scheduler_op *);
     void         (*dump_settings)  (const struct scheduler *);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index b03afb4..4f386f1 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -840,6 +840,9 @@ void scheduler_free(struct scheduler *sched);
 int schedule_cpu_switch(unsigned int cpu, struct cpupool *c);
 void vcpu_force_reschedule(struct vcpu *v);
 int cpu_disable_scheduler(unsigned int cpu);
+/* We need it in dom0_setup_vcpu */
+void sched_set_affinity(struct vcpu *v, const cpumask_t *hard,
+                        const cpumask_t *soft);
 int vcpu_set_hard_affinity(struct vcpu *v, const cpumask_t *affinity);
 int vcpu_set_soft_affinity(struct vcpu *v, const cpumask_t *affinity);
 void restore_vcpu_affinity(struct domain *d);


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

[PATCH 2/4] xen: sched: optimize exclusive pinning case (Credit1 & 2)

[Dario Faggioli]

Exclusive pinning of vCPUs is used, sometimes, for
achieving the highest level of determinism, and the
least possible overhead, for the vCPUs in question.

Although static 1:1 pinning is not recommended, for
general use cases, optimizing the tickling code (of
Credit1 and Credit2) is easy and cheap enough, so go
for it.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Anshul Makkar <anshulmakkar@gmail.com>
---
Changes from v1:
- use a flag during runtime, as suggested during review;
- make use of the affinity-change hook, introduced in pevious patch.
---
 xen/common/sched_credit.c    |   35 +++++++++++++++++++++++++++++++++++
 xen/common/sched_credit2.c   |   40 ++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/perfc_defn.h |    1 +
 3 files changed, 76 insertions(+)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 4fdaa08..3efbfc8 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -73,6 +73,7 @@
 #define CSCHED_FLAG_VCPU_PARKED    0x0  /* VCPU over capped credits */
 #define CSCHED_FLAG_VCPU_YIELD     0x1  /* VCPU yielding */
 #define CSCHED_FLAG_VCPU_MIGRATING 0x2  /* VCPU may have moved to a new pcpu */
+#define CSCHED_FLAG_VCPU_PINNED    0x4  /* VCPU can run only on 1 pcpu */
 
 
 /*
@@ -362,6 +363,25 @@ static inline void __runq_tickle(struct csched_vcpu *new)
     idlers_empty = cpumask_empty(&idle_mask);
 
     /*
+     * Exclusive pinning is when a vcpu has hard-affinity with only one
+     * cpu, and there is no other vcpu that has hard-affinity with that
+     * same cpu. This is infrequent, but if it happens, is for achieving
+     * the most possible determinism, and least possible overhead for
+     * the vcpus in question.
+     *
+     * Try to identify the vast majority of these situations, and deal
+     * with them quickly.
+     */
+    if ( unlikely(test_bit(CSCHED_FLAG_VCPU_PINNED, &new->flags) &&
+                  cpumask_test_cpu(cpu, &idle_mask)) )
+    {
+        ASSERT(cpumask_cycle(cpu, new->vcpu->cpu_hard_affinity) == cpu);
+        SCHED_STAT_CRANK(tickled_idle_cpu_excl);
+        __cpumask_set_cpu(cpu, &mask);
+        goto tickle;
+    }
+
+    /*
      * If the pcpu is idle, or there are no idlers and the new
      * vcpu is a higher priority than the old vcpu, run it here.
      *
@@ -457,6 +477,7 @@ static inline void __runq_tickle(struct csched_vcpu *new)
         }
     }
 
+ tickle:
     if ( !cpumask_empty(&mask) )
     {
         if ( unlikely(tb_init_done) )
@@ -1223,6 +1244,19 @@ csched_dom_cntl(
     return rc;
 }
 
+static void
+csched_aff_cntl(const struct scheduler *ops, struct vcpu *v,
+                const cpumask_t *hard, const cpumask_t *soft)
+{
+    struct csched_vcpu *svc = CSCHED_VCPU(v);
+
+    /* Are we becoming exclusively pinned? */
+    if ( cpumask_weight(hard) == 1 )
+        set_bit(CSCHED_FLAG_VCPU_PINNED, &svc->flags);
+    else
+        clear_bit(CSCHED_FLAG_VCPU_PINNED, &svc->flags);
+}
+
 static inline void
 __csched_set_tslice(struct csched_private *prv, unsigned timeslice)
 {
@@ -2270,6 +2304,7 @@ static const struct scheduler sched_credit_def = {
     .yield          = csched_vcpu_yield,
 
     .adjust         = csched_dom_cntl,
+    .adjust_affinity= csched_aff_cntl,
     .adjust_global  = csched_sys_cntl,
 
     .pick_cpu       = csched_cpu_pick,
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 32234ac..e1985fb 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -299,6 +299,12 @@
  */
 #define __CSFLAG_vcpu_yield 4
 #define CSFLAG_vcpu_yield (1U<<__CSFLAG_vcpu_yield)
+/*
+ * CSFLAGS_pinned: this vcpu is currently 'pinned', i.e., has its hard
+ * affinity set to one and only 1 cpu (and, hence, can only run there).
+ */
+#define __CSFLAG_pinned 5
+#define CSFLAG_pinned (1U<<__CSFLAG_pinned)
 
 static unsigned int __read_mostly opt_migrate_resist = 500;
 integer_param("sched_credit2_migrate_resist", opt_migrate_resist);
@@ -1453,6 +1459,26 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
                     (unsigned char *)&d);
     }
 
+    /*
+     * Exclusive pinning is when a vcpu has hard-affinity with only one
+     * cpu, and there is no other vcpu that has hard-affinity with that
+     * same cpu. This is infrequent, but if it happens, is for achieving
+     * the most possible determinism, and least possible overhead for
+     * the vcpus in question.
+     *
+     * Try to identify the vast majority of these situations, and deal
+     * with them quickly.
+     */
+    if ( unlikely((new->flags & CSFLAG_pinned) &&
+                  cpumask_test_cpu(cpu, &rqd->idle) &&
+                  !cpumask_test_cpu(cpu, &rqd->tickled)) )
+    {
+        ASSERT(cpumask_cycle(cpu, new->vcpu->cpu_hard_affinity) == cpu);
+        SCHED_STAT_CRANK(tickled_idle_cpu_excl);
+        ipid = cpu;
+        goto tickle;
+    }
+
     for_each_affinity_balance_step( bs )
     {
         /* Just skip first step, if we don't have a soft affinity */
@@ -2826,6 +2852,19 @@ csched2_dom_cntl(
     return rc;
 }
 
+static void
+csched2_aff_cntl(const struct scheduler *ops, struct vcpu *v,
+                 const cpumask_t *hard, const cpumask_t *soft)
+{
+    struct csched2_vcpu *svc = csched2_vcpu(v);
+
+    /* Are we becoming exclusively pinned? */
+    if ( cpumask_weight(hard) == 1 )
+        __set_bit(__CSFLAG_pinned, &svc->flags);
+    else
+        __clear_bit(__CSFLAG_pinned, &svc->flags);
+}
+
 static int csched2_sys_cntl(const struct scheduler *ops,
                             struct xen_sysctl_scheduler_op *sc)
 {
@@ -3887,6 +3926,7 @@ static const struct scheduler sched_credit2_def = {
     .yield          = csched2_vcpu_yield,
 
     .adjust         = csched2_dom_cntl,
+    .adjust_affinity= csched2_aff_cntl,
     .adjust_global  = csched2_sys_cntl,
 
     .pick_cpu       = csched2_cpu_pick,
diff --git a/xen/include/xen/perfc_defn.h b/xen/include/xen/perfc_defn.h
index e8ff565..ef6f86b 100644
--- a/xen/include/xen/perfc_defn.h
+++ b/xen/include/xen/perfc_defn.h
@@ -32,6 +32,7 @@ PERFCOUNTER(vcpu_wake_runnable,     "sched: vcpu_wake_runnable")
 PERFCOUNTER(vcpu_wake_not_runnable, "sched: vcpu_wake_not_runnable")
 PERFCOUNTER(tickled_no_cpu,         "sched: tickled_no_cpu")
 PERFCOUNTER(tickled_idle_cpu,       "sched: tickled_idle_cpu")
+PERFCOUNTER(tickled_idle_cpu_excl,  "sched: tickled_idle_cpu_exclusive")
 PERFCOUNTER(tickled_busy_cpu,       "sched: tickled_busy_cpu")
 PERFCOUNTER(vcpu_check,             "sched: vcpu_check")
 


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

[PATCH 3/4] xen: sched: improve checking soft-affinity

[Dario Faggioli]

Whether or not a vCPU has a soft-affinity which is
effective, i.e., with the power of actually affecting
the scheduling of the vCPU itself, happens in an
helper function, called has_soft_affinity().

Such function takes a custom cpumask as its third
parameter, for better flexibility, but that mask is
different from the vCPU's hard-affinity only in one
case. Getting rid of that parameter, not only simplify
the function, but enables for optimizing the soft
affinity check (which will happen, in a subsequent
commit).

This commit, therefore, does that. It's mostly
mechanical, with the only exception _csched_cpu_pick()
(in Credit1 code).

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Anshul Makkar <anshulmakkar@gmail.com>
---
 xen/common/sched_credit.c  |   79 +++++++++++++++++++++-----------------------
 xen/common/sched_credit2.c |   10 ++----
 xen/common/sched_null.c    |    8 ++--
 xen/include/xen/sched-if.h |    8 ++--
 4 files changed, 48 insertions(+), 57 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 3efbfc8..35d0c98 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -410,8 +410,7 @@ static inline void __runq_tickle(struct csched_vcpu *new)
             int new_idlers_empty;
 
             if ( balance_step == BALANCE_SOFT_AFFINITY
-                 && !has_soft_affinity(new->vcpu,
-                                       new->vcpu->cpu_hard_affinity) )
+                 && !has_soft_affinity(new->vcpu) )
                 continue;
 
             /* Are there idlers suitable for new (for this balance step)? */
@@ -743,50 +742,42 @@ __csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
 static int
 _csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc, bool_t commit)
 {
-    cpumask_t cpus;
     cpumask_t idlers;
-    cpumask_t *online;
+    cpumask_t *online = cpupool_domain_cpumask(vc->domain);
     struct csched_pcpu *spc = NULL;
     int cpu = vc->processor;
     int balance_step;
 
-    /* Store in cpus the mask of online cpus on which the domain can run */
-    online = cpupool_domain_cpumask(vc->domain);
-    cpumask_and(&cpus, vc->cpu_hard_affinity, online);
-
     for_each_affinity_balance_step( balance_step )
     {
+        affinity_balance_cpumask(vc, balance_step, cpumask_scratch_cpu(cpu));
+        cpumask_and(cpumask_scratch_cpu(cpu), online, cpumask_scratch_cpu(cpu));
         /*
          * We want to pick up a pcpu among the ones that are online and
-         * can accommodate vc, which is basically what we computed above
-         * and stored in cpus. As far as hard affinity is concerned,
-         * there always will be at least one of these pcpus, hence cpus
-         * is never empty and the calls to cpumask_cycle() and
-         * cpumask_test_cpu() below are ok.
+         * can accommodate vc. As far as hard affinity is concerned, there
+         * always will be at least one of these pcpus in the scratch cpumask,
+         * hence, the calls to cpumask_cycle() and cpumask_test_cpu() below
+         * are ok.
          *
-         * On the other hand, when considering soft affinity too, it
-         * is possible for the mask to become empty (for instance, if the
-         * domain has been put in a cpupool that does not contain any of the
-         * pcpus in its soft affinity), which would result in the ASSERT()-s
-         * inside cpumask_*() operations triggering (in debug builds).
+         * On the other hand, when considering soft affinity, it is possible
+         * that the mask is empty (for instance, if the domain has been put
+         * in a cpupool that does not contain any of the pcpus in its soft
+         * affinity), which would result in the ASSERT()-s inside cpumask_*()
+         * operations triggering (in debug builds).
          *
-         * Therefore, in this case, we filter the soft affinity mask against
-         * cpus and, if the result is empty, we just skip the soft affinity
+         * Therefore, if that is the case, we just skip the soft affinity
          * balancing step all together.
          */
-        if ( balance_step == BALANCE_SOFT_AFFINITY
-             && !has_soft_affinity(vc, &cpus) )
+        if ( balance_step == BALANCE_SOFT_AFFINITY &&
+             (!has_soft_affinity(vc) ||
+              cpumask_empty(cpumask_scratch_cpu(cpu))) )
             continue;
 
-        /* Pick an online CPU from the proper affinity mask */
-        affinity_balance_cpumask(vc, balance_step, &cpus);
-        cpumask_and(&cpus, &cpus, online);
-
         /* If present, prefer vc's current processor */
-        cpu = cpumask_test_cpu(vc->processor, &cpus)
+        cpu = cpumask_test_cpu(vc->processor, cpumask_scratch_cpu(cpu))
                 ? vc->processor
-                : cpumask_cycle(vc->processor, &cpus);
-        ASSERT(cpumask_test_cpu(cpu, &cpus));
+                : cpumask_cycle(vc->processor, cpumask_scratch_cpu(cpu));
+        ASSERT(cpumask_test_cpu(cpu, cpumask_scratch_cpu(cpu)));
 
         /*
          * Try to find an idle processor within the above constraints.
@@ -807,7 +798,8 @@ _csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc, bool_t commit)
         cpumask_and(&idlers, &cpu_online_map, CSCHED_PRIV(ops)->idlers);
         if ( vc->processor == cpu && is_runq_idle(cpu) )
             __cpumask_set_cpu(cpu, &idlers);
-        cpumask_and(&cpus, &cpus, &idlers);
+        cpumask_and(cpumask_scratch_cpu(cpu), &idlers,
+                    cpumask_scratch_cpu(cpu));
 
         /*
          * It is important that cpu points to an idle processor, if a suitable
@@ -821,18 +813,19 @@ _csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc, bool_t commit)
          * Notice that cpumask_test_cpu() is quicker than cpumask_empty(), so
          * we check for it first.
          */
-        if ( !cpumask_test_cpu(cpu, &cpus) && !cpumask_empty(&cpus) )
-            cpu = cpumask_cycle(cpu, &cpus);
-        __cpumask_clear_cpu(cpu, &cpus);
+        if ( !cpumask_test_cpu(cpu, cpumask_scratch_cpu(cpu)) &&
+             !cpumask_empty(cpumask_scratch_cpu(cpu)) )
+            cpu = cpumask_cycle(cpu, cpumask_scratch_cpu(cpu));
+        __cpumask_clear_cpu(cpu, cpumask_scratch_cpu(cpu));
 
-        while ( !cpumask_empty(&cpus) )
+        while ( !cpumask_empty(cpumask_scratch_cpu(cpu)) )
         {
             cpumask_t cpu_idlers;
             cpumask_t nxt_idlers;
             int nxt, weight_cpu, weight_nxt;
             int migrate_factor;
 
-            nxt = cpumask_cycle(cpu, &cpus);
+            nxt = cpumask_cycle(cpu, cpumask_scratch_cpu(cpu));
 
             if ( cpumask_test_cpu(cpu, per_cpu(cpu_core_mask, nxt)) )
             {
@@ -862,14 +855,19 @@ _csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc, bool_t commit)
                  weight_cpu > weight_nxt :
                  weight_cpu * migrate_factor < weight_nxt )
             {
-                cpumask_and(&nxt_idlers, &cpus, &nxt_idlers);
+                cpumask_and(&nxt_idlers, &nxt_idlers,
+                            cpumask_scratch_cpu(cpu));
                 spc = CSCHED_PCPU(nxt);
                 cpu = cpumask_cycle(spc->idle_bias, &nxt_idlers);
-                cpumask_andnot(&cpus, &cpus, per_cpu(cpu_sibling_mask, cpu));
+                cpumask_andnot(cpumask_scratch_cpu(cpu),
+                               cpumask_scratch_cpu(cpu),
+                               per_cpu(cpu_sibling_mask, cpu));
             }
             else
             {
-                cpumask_andnot(&cpus, &cpus, &nxt_idlers);
+                cpumask_andnot(cpumask_scratch_cpu(cpu),
+                               cpumask_scratch_cpu(cpu),
+                               &nxt_idlers);
             }
         }
 
@@ -1687,9 +1685,8 @@ csched_runq_steal(int peer_cpu, int cpu, int pri, int balance_step)
          * vCPUs with useful soft affinities in some sort of bitmap
          * or counter.
          */
-        if ( vc->is_running ||
-             (balance_step == BALANCE_SOFT_AFFINITY
-              && !has_soft_affinity(vc, vc->cpu_hard_affinity)) )
+        if ( vc->is_running || (balance_step == BALANCE_SOFT_AFFINITY &&
+                                !has_soft_affinity(vc)) )
             continue;
 
         affinity_balance_cpumask(vc, balance_step, cpumask_scratch);
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index e1985fb..b5611c9 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -698,8 +698,7 @@ static int get_fallback_cpu(struct csched2_vcpu *svc)
     {
         int cpu = v->processor;
 
-        if ( bs == BALANCE_SOFT_AFFINITY &&
-             !has_soft_affinity(v, v->cpu_hard_affinity) )
+        if ( bs == BALANCE_SOFT_AFFINITY && !has_soft_affinity(v) )
             continue;
 
         affinity_balance_cpumask(v, bs, cpumask_scratch_cpu(cpu));
@@ -1482,8 +1481,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
     for_each_affinity_balance_step( bs )
     {
         /* Just skip first step, if we don't have a soft affinity */
-        if ( bs == BALANCE_SOFT_AFFINITY &&
-             !has_soft_affinity(new->vcpu, new->vcpu->cpu_hard_affinity) )
+        if ( bs == BALANCE_SOFT_AFFINITY && !has_soft_affinity(new->vcpu) )
             continue;
 
         affinity_balance_cpumask(new->vcpu, bs, cpumask_scratch_cpu(cpu));
@@ -2283,7 +2281,7 @@ csched2_cpu_pick(const struct scheduler *ops, struct vcpu *vc)
      *
      * Find both runqueues in one pass.
      */
-    has_soft = has_soft_affinity(vc, vc->cpu_hard_affinity);
+    has_soft = has_soft_affinity(vc);
     for_each_cpu(i, &prv->active_queues)
     {
         struct csched2_runqueue_data *rqd;
@@ -3190,7 +3188,7 @@ runq_candidate(struct csched2_runqueue_data *rqd,
     }
 
     /* If scurr has a soft-affinity, let's check whether cpu is part of it */
-    if ( has_soft_affinity(scurr->vcpu, scurr->vcpu->cpu_hard_affinity) )
+    if ( has_soft_affinity(scurr->vcpu) )
     {
         affinity_balance_cpumask(scurr->vcpu, BALANCE_SOFT_AFFINITY,
                                  cpumask_scratch);
diff --git a/xen/common/sched_null.c b/xen/common/sched_null.c
index b4a24ba..4115d4a 100644
--- a/xen/common/sched_null.c
+++ b/xen/common/sched_null.c
@@ -299,8 +299,7 @@ static unsigned int pick_cpu(struct null_private *prv, struct vcpu *v)
 
     for_each_affinity_balance_step( bs )
     {
-        if ( bs == BALANCE_SOFT_AFFINITY &&
-             !has_soft_affinity(v, v->cpu_hard_affinity) )
+        if ( bs == BALANCE_SOFT_AFFINITY && !has_soft_affinity(v) )
             continue;
 
         affinity_balance_cpumask(v, bs, cpumask_scratch_cpu(cpu));
@@ -512,8 +511,7 @@ static void _vcpu_remove(struct null_private *prv, struct vcpu *v)
     {
         list_for_each_entry( wvc, &prv->waitq, waitq_elem )
         {
-            if ( bs == BALANCE_SOFT_AFFINITY &&
-                 !has_soft_affinity(wvc->vcpu, wvc->vcpu->cpu_hard_affinity) )
+            if ( bs == BALANCE_SOFT_AFFINITY && !has_soft_affinity(wvc->vcpu) )
                 continue;
 
             if ( vcpu_check_affinity(wvc->vcpu, cpu, bs) )
@@ -782,7 +780,7 @@ static struct task_slice null_schedule(const struct scheduler *ops,
             list_for_each_entry( wvc, &prv->waitq, waitq_elem )
             {
                 if ( bs == BALANCE_SOFT_AFFINITY &&
-                     !has_soft_affinity(wvc->vcpu, wvc->vcpu->cpu_hard_affinity) )
+                     !has_soft_affinity(wvc->vcpu) )
                     continue;
 
                 if ( vcpu_check_affinity(wvc->vcpu, cpu, bs) )
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 982c780..417789a 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -243,16 +243,14 @@ static inline cpumask_t* cpupool_domain_cpumask(struct domain *d)
  * 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.
+ * * There is an overlap between the soft and hard affinity masks
  */
-static inline int has_soft_affinity(const struct vcpu *v,
-                                    const cpumask_t *mask)
+static inline int has_soft_affinity(const struct vcpu *v)
 {
     return !cpumask_subset(cpupool_domain_cpumask(v->domain),
                            v->cpu_soft_affinity) &&
            !cpumask_subset(v->cpu_hard_affinity, v->cpu_soft_affinity) &&
-           cpumask_intersects(v->cpu_soft_affinity, mask);
+           cpumask_intersects(v->cpu_soft_affinity, v->cpu_hard_affinity);
 }
 
 /*


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

[PATCH 4/4] xen: sched: simplify (and speedup) checking soft-affinity

[Dario Faggioli]

The fact of whether or not a vCPU has a soft-affinity
which is effective, i.e., with the power of actually
affecting the scheduling of the vCPU itself rarely
changes. Very, very rarely, as compared to how often
we need to check for the same thing (basically, at
every scheduling decision!).

That can be improved by storing in a (per-vCPU) flag
(it's actually a boolean field in struct vcpu) whether
or not, considering how hard-affinity and soft-affinity
look like, soft-affinity should or not be taken into
account during scheduling decisions.

This saves some cpumask manipulations, which is nice,
considering how frequently they were being done. Note
that we can't get rid of 100% of the cpumask operations
involved in the check, because soft-affinity being
effective or not, not only depends on the relationship
between the hard and soft-affinity masks of a vCPU, but
also of the online pCPUs and/or of what pCPUs are part
of the cpupool where the vCPU lives, and that's rather
impractical to store in a per-vCPU flag. Still the
overhead is reduced to "just" one cpumask_subset() (and
only if the newly introduced flag is 'true')!

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Anshul Makkar <anshulmakkar@gmail.com>
---
 xen/common/schedule.c      |    5 +++++
 xen/include/xen/sched-if.h |    7 +++----
 xen/include/xen/sched.h    |    2 ++
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index a8b82fd..284df66 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -854,6 +854,11 @@ void sched_set_affinity(
         cpumask_copy(v->cpu_hard_affinity, hard);
     if ( soft )
         cpumask_copy(v->cpu_hard_affinity, soft);
+
+    v->soft_aff_effective = !cpumask_subset(v->cpu_hard_affinity,
+                                            v->cpu_soft_affinity) &&
+                            cpumask_intersects(v->cpu_soft_affinity,
+                                               v->cpu_hard_affinity);
 }
 
 static int vcpu_set_affinity(
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 417789a..1f4ff1b 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -247,10 +247,9 @@ static inline cpumask_t* cpupool_domain_cpumask(struct domain *d)
  */
 static inline int has_soft_affinity(const struct vcpu *v)
 {
-    return !cpumask_subset(cpupool_domain_cpumask(v->domain),
-                           v->cpu_soft_affinity) &&
-           !cpumask_subset(v->cpu_hard_affinity, v->cpu_soft_affinity) &&
-           cpumask_intersects(v->cpu_soft_affinity, v->cpu_hard_affinity);
+    return v->soft_aff_effective &&
+           !cpumask_subset(cpupool_domain_cpumask(v->domain),
+                           v->cpu_soft_affinity);
 }
 
 /*
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 4f386f1..68c66f3 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -210,6 +210,8 @@ struct vcpu
     bool             hcall_compat;
 #endif
 
+    /* Does soft affinity actually play a role (given hard affinity)? */
+    bool             soft_aff_effective;
 
     /*
      * > 0: a single port is being polled;


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

Re: [PATCH 0/4] xen: sched: optimize exclusive pinning and soft-affinity checking

[Anshul Makkar]

On 9/15/17 6:35 PM, Dario Faggioli wrote:
> Hello,
>
> Dario Faggioli (4):
>        xen: sched: introduce 'adjust_affinity' hook.
>        xen: sched: optimize exclusive pinning case (Credit1 & 2)
>        xen: sched: improve checking soft-affinity
>        xen: sched: simplify (and speedup) checking soft-affinity
>
>   xen/arch/x86/dom0_build.c    |    4 +
>   xen/common/sched_credit.c    |  114 +++++++++++++++++++++++++++---------------
>   xen/common/sched_credit2.c   |   50 ++++++++++++++++--
>   xen/common/sched_null.c      |    8 +--
>   xen/common/schedule.c        |   67 +++++++++++++++++++------
>   xen/include/xen/perfc_defn.h |    1
>   xen/include/xen/sched-if.h   |   16 +++---
>   xen/include/xen/sched.h      |    5 ++
>   8 files changed, 188 insertions(+), 77 deletions(-)
> --
Please can you share the link  for the branch on which these changes 
have been done .

Anshul

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

Re: [PATCH 0/4] xen: sched: optimize exclusive pinning and soft-affinity checking

[Dario Faggioli]

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

On Mon, 2017-09-18 at 20:42 +0100, Anshul Makkar wrote:
> 
> On 9/15/17 6:35 PM, Dario Faggioli wrote:
> > Hello,
> > 
> > Dario Faggioli (4):
> >        xen: sched: introduce 'adjust_affinity' hook.
> >        xen: sched: optimize exclusive pinning case (Credit1 & 2)
> >        xen: sched: improve checking soft-affinity
> >        xen: sched: simplify (and speedup) checking soft-affinity
> > 
> Please can you share the link  for the branch on which these changes 
> have been done .
> 
Hi,

Here it is:

git://xenbits.xen.org/people/dariof/xen.git rel/sched/optim-pin-aff-credit1-2
http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/sched/optim-pin-aff-credit1-2

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: 819 bytes --]

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

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

Re: [PATCH 2/4] xen: sched: optimize exclusive pinning case (Credit1 & 2)

[Anshul Makkar]

On 9/15/17 6:35 PM, Dario Faggioli wrote:
>   
>   static unsigned int __read_mostly opt_migrate_resist = 500;
>   integer_param("sched_credit2_migrate_resist", opt_migrate_resist);
> @@ -1453,6 +1459,26 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
>                       (unsigned char *)&d);
>       }
>   
> +    /*
> +     * Exclusive pinning is when a vcpu has hard-affinity with only one
> +     * cpu, and there is no other vcpu that has hard-affinity with that
> +     * same cpu. This is infrequent, but if it happens, is for achieving
> +     * the most possible determinism, and least possible overhead for
> +     * the vcpus in question.
> +     *
> +     * Try to identify the vast majority of these situations, and deal
> +     * with them quickly.
Sorry, if I have missed to review the earlier series on the same subject.
But, I am not completely satisfied with the condition that pinning to a
pcpu is only possible if and only if no other vcpu has hard affinity of 
that pcpu.
I think we can do away with this condition and give pinning a priority.
Your thoughts please..
> +     */
> +    if ( unlikely((new->flags & CSFLAG_pinned) &&
> +                  cpumask_test_cpu(cpu, &rqd->idle) &&
> +                  !cpumask_test_cpu(cpu, &rqd->tickled)) )
> +    {
> +        ASSERT(cpumask_cycle(cpu, new->vcpu->cpu_hard_affinity) == cpu);
> +        SCHED_STAT_CRANK(tickled_idle_cpu_excl);
> +        ipid = cpu;
> +        goto tickle;
> +    }
> +
>       for_each_affinity_balance_step( bs )
>       {
>           /* Just skip first step, if we don't have a soft affinity */
> @@ -2826,6 +2852,19 @@ csched2_dom_cntl(
>       return rc;
>   }
>   
> +static void
> +csched2_aff_cntl(const struct scheduler *ops, struct vcpu *v,
> +                 const cpumask_t *hard, const cpumask_t *soft)
> +{
> +    struct csched2_vcpu *svc = csched2_vcpu(v);
> +
> +    /* Are we becoming exclusively pinned? */
> +    if ( cpumask_weight(hard) == 1 )
> +        __set_bit(__CSFLAG_pinned, &svc->flags);
> +    else
> +        __clear_bit(__CSFLAG_pinned, &svc->flags);
> +}
> +
>   static int csched2_sys_cntl(const struct scheduler *ops,
>                               struct xen_sysctl_scheduler_op *sc)
Looks fine.

Anshul


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

Re: [PATCH 3/4] xen: sched: improve checking soft-affinity

[George Dunlap]

On 09/15/2017 06:35 PM, Dario Faggioli wrote:
> Whether or not a vCPU has a soft-affinity which is
> effective, i.e., with the power of actually affecting
> the scheduling of the vCPU itself, happens in an
> helper function, called has_soft_affinity().
> 
> Such function takes a custom cpumask as its third
> parameter, for better flexibility, but that mask is
> different from the vCPU's hard-affinity only in one
> case. Getting rid of that parameter, not only simplify
> the function, but enables for optimizing the soft
> affinity check (which will happen, in a subsequent
> commit).
> 
> This commit, therefore, does that. It's mostly
> mechanical, with the only exception _csched_cpu_pick()
> (in Credit1 code).
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> ---
> Cc: George Dunlap <george.dunlap@citrix.com>
> Cc: Anshul Makkar <anshulmakkar@gmail.com>
> ---
>  xen/common/sched_credit.c  |   79 +++++++++++++++++++++-----------------------
>  xen/common/sched_credit2.c |   10 ++----
>  xen/common/sched_null.c    |    8 ++--
>  xen/include/xen/sched-if.h |    8 ++--
>  4 files changed, 48 insertions(+), 57 deletions(-)
> 
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index 3efbfc8..35d0c98 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -410,8 +410,7 @@ static inline void __runq_tickle(struct csched_vcpu *new)
>              int new_idlers_empty;
>  
>              if ( balance_step == BALANCE_SOFT_AFFINITY
> -                 && !has_soft_affinity(new->vcpu,
> -                                       new->vcpu->cpu_hard_affinity) )
> +                 && !has_soft_affinity(new->vcpu) )
>                  continue;
>  
>              /* Are there idlers suitable for new (for this balance step)? */
> @@ -743,50 +742,42 @@ __csched_vcpu_is_migrateable(struct vcpu *vc, int dest_cpu, cpumask_t *mask)
>  static int
>  _csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc, bool_t commit)
>  {
> -    cpumask_t cpus;

Is there a reason you couldn't use cpumask_t *cpus=cpumask_scratch_cpu()?

Other than that, looks good.

 -George

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

Re: [PATCH 1/4] xen: sched: introduce 'adjust_affinity' hook.

[George Dunlap]

On 09/15/2017 06:35 PM, Dario Faggioli wrote:
> For now, just as a way to give a scheduler an "heads up",
> about the fact that the affinity changed.
> 
> This enables some optimizations, such as pre-computing
> and storing (e.g., in flags) facts like a vcpu being
> exclusively pinned to a pcpu, or having or not a
> soft affinity. I.e., conditions that, despite the fact
> that they rarely change, are right now checked very
> frequently, even in hot paths.
> 
> Note also that this, in future, may turn out as a useful
> mean for, e.g., having the schedulers vet, ack or nack
> the changes themselves.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

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

Re: [PATCH 2/4] xen: sched: optimize exclusive pinning case (Credit1 & 2)

[George Dunlap]

On 09/15/2017 06:35 PM, Dario Faggioli wrote:
> Exclusive pinning of vCPUs is used, sometimes, for
> achieving the highest level of determinism, and the
> least possible overhead, for the vCPUs in question.
> 
> Although static 1:1 pinning is not recommended, for
> general use cases, optimizing the tickling code (of
> Credit1 and Credit2) is easy and cheap enough, so go
> for it.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

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

Re: [PATCH 4/4] xen: sched: simplify (and speedup) checking soft-affinity

[George Dunlap]

On 09/15/2017 06:35 PM, Dario Faggioli wrote:
> The fact of whether or not a vCPU has a soft-affinity
> which is effective, i.e., with the power of actually
> affecting the scheduling of the vCPU itself rarely
> changes. Very, very rarely, as compared to how often
> we need to check for the same thing (basically, at
> every scheduling decision!).
> 
> That can be improved by storing in a (per-vCPU) flag
> (it's actually a boolean field in struct vcpu) whether
> or not, considering how hard-affinity and soft-affinity
> look like, soft-affinity should or not be taken into
> account during scheduling decisions.
> 
> This saves some cpumask manipulations, which is nice,
> considering how frequently they were being done. Note
> that we can't get rid of 100% of the cpumask operations
> involved in the check, because soft-affinity being
> effective or not, not only depends on the relationship
> between the hard and soft-affinity masks of a vCPU, but
> also of the online pCPUs and/or of what pCPUs are part
> of the cpupool where the vCPU lives, and that's rather
> impractical to store in a per-vCPU flag. Still the
> overhead is reduced to "just" one cpumask_subset() (and
> only if the newly introduced flag is 'true')!
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

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

Re: [PATCH 0/4] xen: sched: optimize exclusive pinning and soft-affinity checking

[George Dunlap]

On 09/15/2017 06:35 PM, Dario Faggioli wrote:
> Hello,
> 
> This series is a rework of a patch that was, originally, sent as part of a
> series (which went in already):
> 
>  https://lists.xen.org/archives/html/xen-devel/2017-07/msg02167.html
> 
> As it can be seen in the message above, George suggested doing things a little
> bit differently, and I agreed. however, that:
> - require a bit more of rework than expected, but here we are;
> - opened the possibility for even more optimization. :-)
> 
> Basically, the effect of the series is that:
> 1) when a vCPU is exclusively pinned to a pCPU, a lot of checks, while making
>    scheduling decisions in Credit1 and Credit2, are skipped (patch 2);
> 2) the check to see whether or not a vCPU has a soft-affinity that actually
>    matters for the scheduling of the vCPU itself, and should be considered is
>    optimized and made faster (patch 4).
> 
> So, the important bits are in patches 2 and 4, but both patches 1 and 3 are
> necessary to make the other twos possible.

I like it -- thanks, Dario!

 -George

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

Re: [PATCH 3/4] xen: sched: improve checking soft-affinity

[Dario Faggioli]

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

On Wed, 2017-10-04 at 14:23 +0100, George Dunlap wrote:
> On 09/15/2017 06:35 PM, Dario Faggioli wrote:
> > 
> > diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> > index 3efbfc8..35d0c98 100644
> > --- a/xen/common/sched_credit.c
> > +++ b/xen/common/sched_credit.c
> > @@ -410,8 +410,7 @@ static inline void __runq_tickle(struct
> > csched_vcpu *new)
> >              int new_idlers_empty;
> >  
> >              if ( balance_step == BALANCE_SOFT_AFFINITY
> > -                 && !has_soft_affinity(new->vcpu,
> > -                                       new->vcpu-
> > >cpu_hard_affinity) )
> > +                 && !has_soft_affinity(new->vcpu) )
> >                  continue;
> >  
> >              /* Are there idlers suitable for new (for this balance
> > step)? */
> > @@ -743,50 +742,42 @@ __csched_vcpu_is_migrateable(struct vcpu *vc,
> > int dest_cpu, cpumask_t *mask)
> >  static int
> >  _csched_cpu_pick(const struct scheduler *ops, struct vcpu *vc,
> > bool_t commit)
> >  {
> > -    cpumask_t cpus;
> 
> Is there a reason you couldn't use cpumask_t
> *cpus=cpumask_scratch_cpu()?
> 
I was about to say "yes, of course". But while double checking that, I
realized that the patch is actually wrong.

So, even if not 100% intentional... good catch! :-P

In fact, in this function (_csched_cpu_pick()), cpu potentially changes
here:

    cpu = cpumask_test_cpu(vc->processor, cpumask_scratch_cpu(cpu))
            ? vc->processor
            : cpumask_cycle(vc->processor, cpumask_scratch_cpu(cpu));

and here:

    if ( !cpumask_test_cpu(cpu, cpumask_scratch_cpu(cpu)) &&
         !cpumask_empty(cpumask_scratch_cpu(cpu)) )
        cpu = cpumask_cycle(cpu, cpumask_scratch_cpu(cpu));

And it's therefore not ok to continue to use cpumask_scratch_cpu(cpu).

I now have fixed this, but then, while testing, I discovered more, and
much more serious issues.

I'm not actually sure what happened, but it's quite clear I've made a
mess while testing this series (likely, I must mistakenly have tested a
different version of the series, wrt to that I sent).

So I'll send a v3, with the issues I've found fixed, and this time
properly tested.

Sorry everyone, George in particular. :-(

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: 833 bytes --]

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

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