All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs
@ 2017-01-26  0:29 Dario Faggioli
  2017-01-26  0:29 ` [PATCH 1/9] xen: credit2: improve comments' style and definition of CSFLAG-s Dario Faggioli
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:29 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Anshul Makkar

Hello,

This series contains mostly style or cosmetic fixes for Credit2, with the
following two exceptions:
 - 2 actual fixes for (not so severe) behavioral bugs (patches 5 and 6);
 - some tracing improvements (last 3 patches).

More info on the specific changelogs.

The series is also available as a git tree here:

  git://xenbits.xen.org/people/dariof/xen.git rel/sched/credit2-style-tracing-accounting
  https://travis-ci.org/fdario/xen/builds/195387018

Thanks and Regards,
Dario
---
Dario Faggioli (9):
      xen: credit2: improve comments' style and definition of CSFLAG-s
      xen: credit2: make accessor helpers inline functions instead of macros
      xen: credit2: tidy up functions names by removing leading '__'.
      xen: credit2: group the runq manipulating functions.
      xen: credit2: always mark a tickled pCPU as... tickled!
      xen: credit2: don't miss accounting while doing a credit reset.
      xen/tools: tracing: credits can go negative, so use int.
      xen/tools: tracing: trace (Credit2) runq traversal.
      xen/tools: tracing: always report how long next slice will be.

 tools/xentrace/formats     |    4 
 tools/xentrace/xenalyze.c  |   52 ++-
 xen/common/sched_credit2.c |  903 +++++++++++++++++++++++---------------------
 xen/common/schedule.c      |    4 
 xen/include/public/trace.h |    1 
 5 files changed, 528 insertions(+), 436 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/9] xen: credit2: improve comments' style and definition of CSFLAG-s
  2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
@ 2017-01-26  0:29 ` Dario Faggioli
  2017-01-26  0:30 ` [PATCH 2/9] xen: credit2: make accessor helpers inline functions instead of macros Dario Faggioli
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:29 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Anshul Makkar

Most of the comments describing the meaning of the
vCPU flags used by the scheduler miss the 'wings' (or
have other minor style issues).

Also, use 1U (instead of 1) as the base of shiftings.

No functional change intended.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Anshul Makkar <anshul.makkar@citrix.com>
---
 xen/common/sched_credit2.c |   54 ++++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 20 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 2ce738d..322cf6b 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -129,35 +129,47 @@
 /*
  * Basic constants
  */
-/* Default weight: How much a new domain starts with */
+/* Default weight: How much a new domain starts with. */
 #define CSCHED2_DEFAULT_WEIGHT       256
-/* Min timer: Minimum length a timer will be set, to
- * achieve efficiency */
+/*
+ * Min timer: Minimum length a timer will be set, to
+ * achieve efficiency.
+ */
 #define CSCHED2_MIN_TIMER            MICROSECS(500)
-/* Amount of credit VMs begin with, and are reset to.
+/*
+ * Amount of credit VMs begin with, and are reset to.
  * ATM, set so that highest-weight VMs can only run for 10ms
- * before a reset event. */
+ * before a reset event.
+ */
 #define CSCHED2_CREDIT_INIT          MILLISECS(10)
-/* Carryover: How much "extra" credit may be carried over after
- * a reset. */
+/*
+ * Amount of credit the idle vcpus have. It never changes, as idle
+ * vcpus does not consume credits, and it must be lower than whatever
+ * amount of credit 'regular' vcpu would end up with.
+ */
+#define CSCHED2_IDLE_CREDIT          (-(1U<<30))
+/*
+ * Carryover: How much "extra" credit may be carried over after
+ * a reset.
+ */
 #define CSCHED2_CARRYOVER_MAX        CSCHED2_MIN_TIMER
-/* Stickiness: Cross-L2 migration resistance.  Should be less than
- * MIN_TIMER. */
+/*
+ * Stickiness: Cross-L2 migration resistance.  Should be less than
+ * MIN_TIMER.
+ */
 #define CSCHED2_MIGRATE_RESIST       ((opt_migrate_resist)*MICROSECS(1))
-/* How much to "compensate" a vcpu for L2 migration */
+/* How much to "compensate" a vcpu for L2 migration. */
 #define CSCHED2_MIGRATE_COMPENSATION MICROSECS(50)
 /* Reset: Value below which credit will be reset. */
 #define CSCHED2_CREDIT_RESET         0
 /* Max timer: Maximum time a guest can be run for. */
 #define CSCHED2_MAX_TIMER            CSCHED2_CREDIT_INIT
 
-
-#define CSCHED2_IDLE_CREDIT                 (-(1<<30))
-
 /*
  * Flags
  */
-/* CSFLAG_scheduled: Is this vcpu either running on, or context-switching off,
+/*
+ * CSFLAG_scheduled: Is this vcpu either running on, or context-switching off,
  * a physical cpu?
  * + Accessed only with runqueue lock held
  * + Set when chosen as next in csched2_schedule().
@@ -167,8 +179,9 @@
  * + Checked to be false in runq_insert.
  */
 #define __CSFLAG_scheduled 1
-#define CSFLAG_scheduled (1<<__CSFLAG_scheduled)
-/* CSFLAG_delayed_runq_add: Do we need to add this to the runqueue once it'd done
+#define CSFLAG_scheduled (1U<<__CSFLAG_scheduled)
+/*
+ * CSFLAG_delayed_runq_add: Do we need to add this to the runqueue once it'd done
  * being context switched out?
  * + Set when scheduling out in csched2_schedule() if prev is runnable
  * + Set in csched2_vcpu_wake if it finds CSFLAG_scheduled set
@@ -176,20 +189,21 @@
  *   clears the bit.
  */
 #define __CSFLAG_delayed_runq_add 2
-#define CSFLAG_delayed_runq_add (1<<__CSFLAG_delayed_runq_add)
-/* CSFLAG_runq_migrate_request: This vcpu is being migrated as a result of a
+#define CSFLAG_delayed_runq_add (1U<<__CSFLAG_delayed_runq_add)
+/*
+ * CSFLAG_runq_migrate_request: This vcpu is being migrated as a result of a
  * credit2-initiated runq migrate request; migrate it to the runqueue indicated
  * in the svc struct. 
  */
 #define __CSFLAG_runq_migrate_request 3
-#define CSFLAG_runq_migrate_request (1<<__CSFLAG_runq_migrate_request)
+#define CSFLAG_runq_migrate_request (1U<<__CSFLAG_runq_migrate_request)
 /*
  * CSFLAG_vcpu_yield: this vcpu was running, and has called vcpu_yield(). The
  * scheduler is invoked to see if we can give the cpu to someone else, and
  * get back to the yielding vcpu in a while.
  */
 #define __CSFLAG_vcpu_yield 4
-#define CSFLAG_vcpu_yield (1<<__CSFLAG_vcpu_yield)
+#define CSFLAG_vcpu_yield (1U<<__CSFLAG_vcpu_yield)
 
 static unsigned int __read_mostly opt_migrate_resist = 500;
 integer_param("sched_credit2_migrate_resist", opt_migrate_resist);


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/9] xen: credit2: make accessor helpers inline functions instead of macros
  2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
  2017-01-26  0:29 ` [PATCH 1/9] xen: credit2: improve comments' style and definition of CSFLAG-s Dario Faggioli
@ 2017-01-26  0:30 ` Dario Faggioli
  2017-01-26  0:30 ` [PATCH 3/9] xen: credit2: tidy up functions names by removing leading '__' Dario Faggioli
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:30 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Anshul Makkar

There isn't any particular reason for the accessor helpers
to be macro, so turn them into 'static inline'-s, which are
better.

Note that it is necessary to move the function definitions
below the structure declarations.

No functional change intended.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Anshul Makkar <anshul.makkar@citrix.com>
---
 xen/common/sched_credit2.c |  158 +++++++++++++++++++++++++-------------------
 1 file changed, 90 insertions(+), 68 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 322cf6b..29973cf 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -209,18 +209,6 @@ static unsigned int __read_mostly opt_migrate_resist = 500;
 integer_param("sched_credit2_migrate_resist", opt_migrate_resist);
 
 /*
- * Useful macros
- */
-#define CSCHED2_PRIV(_ops)   \
-    ((struct csched2_private *)((_ops)->sched_data))
-#define CSCHED2_VCPU(_vcpu)  ((struct csched2_vcpu *) (_vcpu)->sched_priv)
-#define CSCHED2_DOM(_dom)    ((struct csched2_dom *) (_dom)->sched_priv)
-/* CPU to runq_id macro */
-#define c2r(_ops, _cpu)     (CSCHED2_PRIV(_ops)->runq_map[(_cpu)])
-/* CPU to runqueue struct macro */
-#define RQD(_ops, _cpu)     (&CSCHED2_PRIV(_ops)->rqd[c2r(_ops, _cpu)])
-
-/*
  * Load tracking and load balancing
  *
  * Load history of runqueues and vcpus is accounted for by using an
@@ -441,6 +429,40 @@ struct csched2_dom {
 };
 
 /*
+ * Accessor helpers functions.
+ */
+static always_inline
+struct csched2_private *csched2_priv(const struct scheduler *ops)
+{
+    return ops->sched_data;
+}
+
+static always_inline
+struct csched2_vcpu *csched2_vcpu(struct vcpu *v)
+{
+    return v->sched_priv;
+}
+
+static always_inline
+struct csched2_dom *csched2_dom(struct domain *d)
+{
+    return d->sched_priv;
+}
+
+/* CPU to runq_id macro */
+static always_inline int c2r(const struct scheduler *ops, unsigned cpu)
+{
+    return (csched2_priv(ops))->runq_map[(cpu)];
+}
+
+/* CPU to runqueue struct macro */
+static always_inline
+struct csched2_runqueue_data *c2rqd(const struct scheduler *ops, unsigned cpu)
+{
+    return &csched2_priv(ops)->rqd[c2r(ops, cpu)];
+}
+
+/*
  * Hyperthreading (SMT) support.
  *
  * We use a special per-runq mask (smt_idle) and update it according to the
@@ -694,7 +716,7 @@ static void
 __update_runq_load(const struct scheduler *ops,
                   struct csched2_runqueue_data *rqd, int change, s_time_t now)
 {
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     s_time_t delta, load = rqd->load;
     unsigned int P, W;
 
@@ -781,7 +803,7 @@ static void
 __update_svc_load(const struct scheduler *ops,
                   struct csched2_vcpu *svc, int change, s_time_t now)
 {
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     s_time_t delta, vcpu_load;
     unsigned int P, W;
 
@@ -878,7 +900,7 @@ static void
 runq_insert(const struct scheduler *ops, struct csched2_vcpu *svc)
 {
     unsigned int cpu = svc->vcpu->processor;
-    struct list_head * runq = &RQD(ops, cpu)->runq;
+    struct list_head * runq = &c2rqd(ops, cpu)->runq;
     int pos = 0;
 
     ASSERT(spin_is_locked(per_cpu(schedule_data, cpu).schedule_lock));
@@ -936,7 +958,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
     int i, ipid = -1;
     s_time_t lowest = (1<<30);
     unsigned int cpu = new->vcpu->processor;
-    struct csched2_runqueue_data *rqd = RQD(ops, cpu);
+    struct csched2_runqueue_data *rqd = c2rqd(ops, cpu);
     cpumask_t mask;
     struct csched2_vcpu * cur;
 
@@ -1006,7 +1028,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
     cpumask_and(&mask, &mask, cpumask_scratch_cpu(cpu));
     if ( cpumask_test_cpu(cpu, &mask) )
     {
-        cur = CSCHED2_VCPU(curr_on_cpu(cpu));
+        cur = csched2_vcpu(curr_on_cpu(cpu));
         burn_credits(rqd, cur, now);
 
         if ( cur->credit < new->credit )
@@ -1023,7 +1045,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
         if ( i == cpu )
             continue;
 
-        cur = CSCHED2_VCPU(curr_on_cpu(i));
+        cur = csched2_vcpu(curr_on_cpu(i));
 
         /*
          * Even if the cpu is not in rqd->idle, it may be running the
@@ -1096,7 +1118,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
 static void reset_credit(const struct scheduler *ops, int cpu, s_time_t now,
                          struct csched2_vcpu *snext)
 {
-    struct csched2_runqueue_data *rqd = RQD(ops, cpu);
+    struct csched2_runqueue_data *rqd = c2rqd(ops, cpu);
     struct list_head *iter;
     int m;
 
@@ -1174,7 +1196,7 @@ void burn_credits(struct csched2_runqueue_data *rqd,
 {
     s_time_t delta;
 
-    ASSERT(svc == CSCHED2_VCPU(curr_on_cpu(svc->vcpu->processor)));
+    ASSERT(svc == csched2_vcpu(curr_on_cpu(svc->vcpu->processor)));
 
     if ( unlikely(is_idle_vcpu(svc->vcpu)) )
     {
@@ -1262,11 +1284,11 @@ static void update_max_weight(struct csched2_runqueue_data *rqd, int new_weight,
 static /*inline*/ void
 __csched2_vcpu_check(struct vcpu *vc)
 {
-    struct csched2_vcpu * const svc = CSCHED2_VCPU(vc);
+    struct csched2_vcpu * const svc = csched2_vcpu(vc);
     struct csched2_dom * const sdom = svc->sdom;
 
     BUG_ON( svc->vcpu != vc );
-    BUG_ON( sdom != CSCHED2_DOM(vc->domain) );
+    BUG_ON( sdom != csched2_dom(vc->domain) );
     if ( sdom )
     {
         BUG_ON( is_idle_vcpu(vc) );
@@ -1306,7 +1328,7 @@ csched2_alloc_vdata(const struct scheduler *ops, struct vcpu *vc, void *dd)
         svc->credit = CSCHED2_CREDIT_INIT;
         svc->weight = svc->sdom->weight;
         /* Starting load of 50% */
-        svc->avgload = 1ULL << (CSCHED2_PRIV(ops)->load_precision_shift - 1);
+        svc->avgload = 1ULL << (csched2_priv(ops)->load_precision_shift - 1);
         svc->load_last_update = NOW() >> LOADAVG_GRANULARITY_SHIFT;
     }
     else
@@ -1358,7 +1380,7 @@ runq_assign(const struct scheduler *ops, struct vcpu *vc)
 
     ASSERT(svc->rqd == NULL);
 
-    __runq_assign(svc, RQD(ops, vc->processor));
+    __runq_assign(svc, c2rqd(ops, vc->processor));
 }
 
 static void
@@ -1383,7 +1405,7 @@ runq_deassign(const struct scheduler *ops, struct vcpu *vc)
 {
     struct csched2_vcpu *svc = vc->sched_priv;
 
-    ASSERT(svc->rqd == RQD(ops, vc->processor));
+    ASSERT(svc->rqd == c2rqd(ops, vc->processor));
 
     __runq_deassign(svc);
 }
@@ -1391,7 +1413,7 @@ runq_deassign(const struct scheduler *ops, struct vcpu *vc)
 static void
 csched2_vcpu_sleep(const struct scheduler *ops, struct vcpu *vc)
 {
-    struct csched2_vcpu * const svc = CSCHED2_VCPU(vc);
+    struct csched2_vcpu * const svc = csched2_vcpu(vc);
 
     ASSERT(!is_idle_vcpu(vc));
     SCHED_STAT_CRANK(vcpu_sleep);
@@ -1400,7 +1422,7 @@ csched2_vcpu_sleep(const struct scheduler *ops, struct vcpu *vc)
         cpu_raise_softirq(vc->processor, SCHEDULE_SOFTIRQ);
     else if ( __vcpu_on_runq(svc) )
     {
-        ASSERT(svc->rqd == RQD(ops, vc->processor));
+        ASSERT(svc->rqd == c2rqd(ops, vc->processor));
         update_load(ops, svc->rqd, svc, -1, NOW());
         __runq_remove(svc);
     }
@@ -1411,7 +1433,7 @@ csched2_vcpu_sleep(const struct scheduler *ops, struct vcpu *vc)
 static void
 csched2_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
 {
-    struct csched2_vcpu * const svc = CSCHED2_VCPU(vc);
+    struct csched2_vcpu * const svc = csched2_vcpu(vc);
     unsigned int cpu = vc->processor;
     s_time_t now;
 
@@ -1449,7 +1471,7 @@ csched2_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
     if ( svc->rqd == NULL )
         runq_assign(ops, vc);
     else
-        ASSERT(RQD(ops, vc->processor) == svc->rqd );
+        ASSERT(c2rqd(ops, vc->processor) == svc->rqd );
 
     now = NOW();
 
@@ -1466,7 +1488,7 @@ out:
 static void
 csched2_vcpu_yield(const struct scheduler *ops, struct vcpu *v)
 {
-    struct csched2_vcpu * const svc = CSCHED2_VCPU(v);
+    struct csched2_vcpu * const svc = csched2_vcpu(v);
 
     __set_bit(__CSFLAG_vcpu_yield, &svc->flags);
 }
@@ -1474,12 +1496,12 @@ csched2_vcpu_yield(const struct scheduler *ops, struct vcpu *v)
 static void
 csched2_context_saved(const struct scheduler *ops, struct vcpu *vc)
 {
-    struct csched2_vcpu * const svc = CSCHED2_VCPU(vc);
+    struct csched2_vcpu * const svc = csched2_vcpu(vc);
     spinlock_t *lock = vcpu_schedule_lock_irq(vc);
     s_time_t now = NOW();
 
-    BUG_ON( !is_idle_vcpu(vc) && svc->rqd != RQD(ops, vc->processor));
-    ASSERT(is_idle_vcpu(vc) || svc->rqd == RQD(ops, vc->processor));
+    BUG_ON( !is_idle_vcpu(vc) && svc->rqd != c2rqd(ops, vc->processor));
+    ASSERT(is_idle_vcpu(vc) || svc->rqd == c2rqd(ops, vc->processor));
 
     /* This vcpu is now eligible to be put on the runqueue again */
     __clear_bit(__CSFLAG_scheduled, &svc->flags);
@@ -1510,9 +1532,9 @@ csched2_context_saved(const struct scheduler *ops, struct vcpu *vc)
 static int
 csched2_cpu_pick(const struct scheduler *ops, struct vcpu *vc)
 {
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     int i, min_rqi = -1, new_cpu, cpu = vc->processor;
-    struct csched2_vcpu *svc = CSCHED2_VCPU(vc);
+    struct csched2_vcpu *svc = csched2_vcpu(vc);
     s_time_t min_avgload = MAX_LOAD;
 
     ASSERT(!cpumask_empty(&prv->active_queues));
@@ -1775,7 +1797,7 @@ static bool_t vcpu_is_migrateable(struct csched2_vcpu *svc,
 
 static void balance_load(const struct scheduler *ops, int cpu, s_time_t now)
 {
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     int i, max_delta_rqi = -1;
     struct list_head *push_iter, *pull_iter;
     bool_t inner_load_updated = 0;
@@ -1790,7 +1812,7 @@ static void balance_load(const struct scheduler *ops, int cpu, s_time_t now)
      */
 
     ASSERT(spin_is_locked(per_cpu(schedule_data, cpu).schedule_lock));
-    st.lrqd = RQD(ops, cpu);
+    st.lrqd = c2rqd(ops, cpu);
 
     __update_runq_load(ops, st.lrqd, 0, now);
 
@@ -1961,12 +1983,12 @@ csched2_vcpu_migrate(
     const struct scheduler *ops, struct vcpu *vc, unsigned int new_cpu)
 {
     struct domain *d = vc->domain;
-    struct csched2_vcpu * const svc = CSCHED2_VCPU(vc);
+    struct csched2_vcpu * const svc = csched2_vcpu(vc);
     struct csched2_runqueue_data *trqd;
     s_time_t now = NOW();
 
     /* Check if new_cpu is valid */
-    ASSERT(cpumask_test_cpu(new_cpu, &CSCHED2_PRIV(ops)->initialized));
+    ASSERT(cpumask_test_cpu(new_cpu, &csched2_priv(ops)->initialized));
     ASSERT(cpumask_test_cpu(new_cpu, vc->cpu_hard_affinity));
 
     /*
@@ -1997,7 +2019,7 @@ csched2_vcpu_migrate(
         return;
     }
 
-    trqd = RQD(ops, new_cpu);
+    trqd = c2rqd(ops, new_cpu);
 
     /*
      * Do the actual movement toward new_cpu, and update vc->processor.
@@ -2019,8 +2041,8 @@ csched2_dom_cntl(
     struct domain *d,
     struct xen_domctl_scheduler_op *op)
 {
-    struct csched2_dom * const sdom = CSCHED2_DOM(d);
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_dom * const sdom = csched2_dom(d);
+    struct csched2_private *prv = csched2_priv(ops);
     unsigned long flags;
     int rc = 0;
 
@@ -2053,10 +2075,10 @@ csched2_dom_cntl(
             /* Update weights for vcpus, and max_weight for runqueues on which they reside */
             for_each_vcpu ( d, v )
             {
-                struct csched2_vcpu *svc = CSCHED2_VCPU(v);
+                struct csched2_vcpu *svc = csched2_vcpu(v);
                 spinlock_t *lock = vcpu_schedule_lock(svc->vcpu);
 
-                ASSERT(svc->rqd == RQD(ops, svc->vcpu->processor));
+                ASSERT(svc->rqd == c2rqd(ops, svc->vcpu->processor));
 
                 svc->weight = sdom->weight;
                 update_max_weight(svc->rqd, svc->weight, old_weight);
@@ -2080,7 +2102,7 @@ static int csched2_sys_cntl(const struct scheduler *ops,
                             struct xen_sysctl_scheduler_op *sc)
 {
     xen_sysctl_credit2_schedule_t *params = &sc->u.sched_credit2;
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     unsigned long flags;
 
     switch (sc->cmd )
@@ -2111,7 +2133,7 @@ static int csched2_sys_cntl(const struct scheduler *ops,
 static void *
 csched2_alloc_domdata(const struct scheduler *ops, struct domain *dom)
 {
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     struct csched2_dom *sdom;
     unsigned long flags;
 
@@ -2127,7 +2149,7 @@ csched2_alloc_domdata(const struct scheduler *ops, struct domain *dom)
 
     write_lock_irqsave(&prv->lock, flags);
 
-    list_add_tail(&sdom->sdom_elem, &CSCHED2_PRIV(ops)->sdom);
+    list_add_tail(&sdom->sdom_elem, &csched2_priv(ops)->sdom);
 
     write_unlock_irqrestore(&prv->lock, flags);
 
@@ -2156,7 +2178,7 @@ csched2_free_domdata(const struct scheduler *ops, void *data)
 {
     unsigned long flags;
     struct csched2_dom *sdom = data;
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
 
     write_lock_irqsave(&prv->lock, flags);
 
@@ -2170,9 +2192,9 @@ csched2_free_domdata(const struct scheduler *ops, void *data)
 static void
 csched2_dom_destroy(const struct scheduler *ops, struct domain *dom)
 {
-    ASSERT(CSCHED2_DOM(dom)->nr_vcpus == 0);
+    ASSERT(csched2_dom(dom)->nr_vcpus == 0);
 
-    csched2_free_domdata(ops, CSCHED2_DOM(dom));
+    csched2_free_domdata(ops, csched2_dom(dom));
 }
 
 static void
@@ -2217,7 +2239,7 @@ csched2_free_vdata(const struct scheduler *ops, void *priv)
 static void
 csched2_vcpu_remove(const struct scheduler *ops, struct vcpu *vc)
 {
-    struct csched2_vcpu * const svc = CSCHED2_VCPU(vc);
+    struct csched2_vcpu * const svc = csched2_vcpu(vc);
     spinlock_t *lock;
 
     ASSERT(!is_idle_vcpu(vc));
@@ -2242,9 +2264,9 @@ csched2_runtime(const struct scheduler *ops, int cpu,
 {
     s_time_t time, min_time;
     int rt_credit; /* Proposed runtime measured in credits */
-    struct csched2_runqueue_data *rqd = RQD(ops, cpu);
+    struct csched2_runqueue_data *rqd = c2rqd(ops, cpu);
     struct list_head *runq = &rqd->runq;
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
 
     /*
      * If we're idle, just stay so. Others (or external events)
@@ -2333,7 +2355,7 @@ runq_candidate(struct csched2_runqueue_data *rqd,
 {
     struct list_head *iter;
     struct csched2_vcpu *snext = NULL;
-    struct csched2_private *prv = CSCHED2_PRIV(per_cpu(scheduler, cpu));
+    struct csched2_private *prv = csched2_priv(per_cpu(scheduler, cpu));
     bool yield = __test_and_clear_bit(__CSFLAG_vcpu_yield, &scurr->flags);
 
     *skipped = 0;
@@ -2372,7 +2394,7 @@ runq_candidate(struct csched2_runqueue_data *rqd,
     if ( vcpu_runnable(scurr->vcpu) )
         snext = scurr;
     else
-        snext = CSCHED2_VCPU(idle_vcpu[cpu]);
+        snext = csched2_vcpu(idle_vcpu[cpu]);
 
     list_for_each( iter, &rqd->runq )
     {
@@ -2452,7 +2474,7 @@ csched2_schedule(
 {
     const int cpu = smp_processor_id();
     struct csched2_runqueue_data *rqd;
-    struct csched2_vcpu * const scurr = CSCHED2_VCPU(current);
+    struct csched2_vcpu * const scurr = csched2_vcpu(current);
     struct csched2_vcpu *snext = NULL;
     unsigned int skipped_vcpus = 0;
     struct task_slice ret;
@@ -2461,9 +2483,9 @@ csched2_schedule(
     SCHED_STAT_CRANK(schedule);
     CSCHED2_VCPU_CHECK(current);
 
-    BUG_ON(!cpumask_test_cpu(cpu, &CSCHED2_PRIV(ops)->initialized));
+    BUG_ON(!cpumask_test_cpu(cpu, &csched2_priv(ops)->initialized));
 
-    rqd = RQD(ops, cpu);
+    rqd = c2rqd(ops, cpu);
     BUG_ON(!cpumask_test_cpu(cpu, &rqd->active));
 
     ASSERT(spin_is_locked(per_cpu(schedule_data, cpu).schedule_lock));
@@ -2521,7 +2543,7 @@ csched2_schedule(
     {
         __clear_bit(__CSFLAG_vcpu_yield, &scurr->flags);
         trace_var(TRC_CSCHED2_SCHED_TASKLET, 1, 0, NULL);
-        snext = CSCHED2_VCPU(idle_vcpu[cpu]);
+        snext = csched2_vcpu(idle_vcpu[cpu]);
     }
     else
         snext = runq_candidate(rqd, scurr, cpu, now, &skipped_vcpus);
@@ -2642,7 +2664,7 @@ csched2_dump_vcpu(struct csched2_private *prv, struct csched2_vcpu *svc)
 static void
 csched2_dump_pcpu(const struct scheduler *ops, int cpu)
 {
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     struct list_head *runq, *iter;
     struct csched2_vcpu *svc;
     unsigned long flags;
@@ -2661,7 +2683,7 @@ csched2_dump_pcpu(const struct scheduler *ops, int cpu)
     lock = per_cpu(schedule_data, cpu).schedule_lock;
     spin_lock(lock);
 
-    runq = &RQD(ops, cpu)->runq;
+    runq = &c2rqd(ops, cpu)->runq;
 
     cpumask_scnprintf(cpustr, sizeof(cpustr), per_cpu(cpu_sibling_mask, cpu));
     printk(" sibling=%s, ", cpustr);
@@ -2669,7 +2691,7 @@ csched2_dump_pcpu(const struct scheduler *ops, int cpu)
     printk("core=%s\n", cpustr);
 
     /* current VCPU */
-    svc = CSCHED2_VCPU(curr_on_cpu(cpu));
+    svc = csched2_vcpu(curr_on_cpu(cpu));
     if ( svc )
     {
         printk("\trun: ");
@@ -2696,7 +2718,7 @@ static void
 csched2_dump(const struct scheduler *ops)
 {
     struct list_head *iter_sdom;
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     unsigned long flags;
     int i, loop;
 #define cpustr keyhandler_scratch
@@ -2756,7 +2778,7 @@ csched2_dump(const struct scheduler *ops)
 
         for_each_vcpu( sdom->dom, v )
         {
-            struct csched2_vcpu * const svc = CSCHED2_VCPU(v);
+            struct csched2_vcpu * const svc = csched2_vcpu(v);
             spinlock_t *lock;
 
             lock = vcpu_schedule_lock(svc->vcpu);
@@ -2899,7 +2921,7 @@ init_pdata(struct csched2_private *prv, unsigned int cpu)
 static void
 csched2_init_pdata(const struct scheduler *ops, void *pdata, int cpu)
 {
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     spinlock_t *old_lock;
     unsigned long flags;
     unsigned rqi;
@@ -2927,7 +2949,7 @@ static void
 csched2_switch_sched(struct scheduler *new_ops, unsigned int cpu,
                      void *pdata, void *vdata)
 {
-    struct csched2_private *prv = CSCHED2_PRIV(new_ops);
+    struct csched2_private *prv = csched2_priv(new_ops);
     struct csched2_vcpu *svc = vdata;
     unsigned rqi;
 
@@ -2974,7 +2996,7 @@ static void
 csched2_deinit_pdata(const struct scheduler *ops, void *pcpu, int cpu)
 {
     unsigned long flags;
-    struct csched2_private *prv = CSCHED2_PRIV(ops);
+    struct csched2_private *prv = csched2_priv(ops);
     struct csched2_runqueue_data *rqd;
     int rqi;
 
@@ -3083,7 +3105,7 @@ csched2_deinit(struct scheduler *ops)
 {
     struct csched2_private *prv;
 
-    prv = CSCHED2_PRIV(ops);
+    prv = csched2_priv(ops);
     ops->sched_data = NULL;
     xfree(prv);
 }


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/9] xen: credit2: tidy up functions names by removing leading '__'.
  2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
  2017-01-26  0:29 ` [PATCH 1/9] xen: credit2: improve comments' style and definition of CSFLAG-s Dario Faggioli
  2017-01-26  0:30 ` [PATCH 2/9] xen: credit2: make accessor helpers inline functions instead of macros Dario Faggioli
@ 2017-01-26  0:30 ` Dario Faggioli
  2017-01-26  0:30 ` [PATCH 4/9] xen: credit2: group the runq manipulating functions Dario Faggioli
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:30 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Anshul Makkar

There is no reason for having pretty much all of the
functions whose names begin with double underscores
('__') to actually look like that.

In fact, that is misleading and makes the code hard
to read and understand. So, remove the '__'-s.

The only two that we keep are __runq_insert() and
__runq_assign() (althought they're converted to
single underscore). In fact, in those cases, it is
indeed useful to have those sort of a "raw" variants.

In case of __runq_insert(), which is only called
once, by runq_insert(), merge the two functions.

No functional change intended.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Anshul Makkar <anshul.makkar@citrix.com>
---
 xen/common/sched_credit2.c |  114 +++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 65 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 29973cf..b764cf9 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -221,7 +221,7 @@ integer_param("sched_credit2_migrate_resist", opt_migrate_resist);
  * shift all time samples to the right.
  *
  * The details of the formulas used for load tracking are explained close to
- * __update_runq_load(). Let's just say here that, with full nanosecond time
+ * update_runq_load(). Let's just say here that, with full nanosecond time
  * granularity, a 30 bits wide 'decaying window' is ~1 second long.
  *
  * We want to consider the following equations:
@@ -233,7 +233,7 @@ integer_param("sched_credit2_migrate_resist", opt_migrate_resist);
  * Q-format fixed point arithmetic and load is the instantaneous load of a
  * runqueue, which basically is the number of runnable vcpus there are on the
  * runqueue (for the meaning of the other terms, look at the doc comment to
- *  __update_runq_load()).
+ *  update_runq_load()).
  *
  *  So, again, with full nanosecond granularity, and 1 second window, we have:
  *
@@ -594,14 +594,12 @@ static s_time_t c2t(struct csched2_runqueue_data *rqd, s_time_t credit, struct c
  * Runqueue related code
  */
 
-static /*inline*/ int
-__vcpu_on_runq(struct csched2_vcpu *svc)
+static inline int vcpu_on_runq(struct csched2_vcpu *svc)
 {
     return !list_empty(&svc->runq_elem);
 }
 
-static /*inline*/ struct csched2_vcpu *
-__runq_elem(struct list_head *elem)
+static struct csched2_vcpu * runq_elem(struct list_head *elem)
 {
     return list_entry(elem, struct csched2_vcpu, runq_elem);
 }
@@ -713,8 +711,8 @@ __runq_elem(struct list_head *elem)
  * Which, in both cases, is what we expect.
  */
 static void
-__update_runq_load(const struct scheduler *ops,
-                  struct csched2_runqueue_data *rqd, int change, s_time_t now)
+update_runq_load(const struct scheduler *ops,
+                 struct csched2_runqueue_data *rqd, int change, s_time_t now)
 {
     struct csched2_private *prv = csched2_priv(ops);
     s_time_t delta, load = rqd->load;
@@ -800,8 +798,8 @@ __update_runq_load(const struct scheduler *ops,
 }
 
 static void
-__update_svc_load(const struct scheduler *ops,
-                  struct csched2_vcpu *svc, int change, s_time_t now)
+update_svc_load(const struct scheduler *ops,
+                struct csched2_vcpu *svc, int change, s_time_t now)
 {
     struct csched2_private *prv = csched2_priv(ops);
     s_time_t delta, vcpu_load;
@@ -865,17 +863,24 @@ update_load(const struct scheduler *ops,
 {
     trace_var(TRC_CSCHED2_UPDATE_LOAD, 1, 0,  NULL);
 
-    __update_runq_load(ops, rqd, change, now);
+    update_runq_load(ops, rqd, change, now);
     if ( svc )
-        __update_svc_load(ops, svc, change, now);
+        update_svc_load(ops, svc, change, now);
 }
 
-static int
-__runq_insert(struct list_head *runq, struct csched2_vcpu *svc)
+static void
+runq_insert(const struct scheduler *ops, struct csched2_vcpu *svc)
 {
     struct list_head *iter;
+    unsigned int cpu = svc->vcpu->processor;
+    struct list_head * runq = &c2rqd(ops, cpu)->runq;
     int pos = 0;
 
+    ASSERT(spin_is_locked(per_cpu(schedule_data, cpu).schedule_lock));
+
+    ASSERT(!vcpu_on_runq(svc));
+    ASSERT(c2r(ops, cpu) == c2r(ops, svc->vcpu->processor));
+
     ASSERT(&svc->rqd->runq == runq);
     ASSERT(!is_idle_vcpu(svc->vcpu));
     ASSERT(!svc->vcpu->is_running);
@@ -883,33 +888,15 @@ __runq_insert(struct list_head *runq, struct csched2_vcpu *svc)
 
     list_for_each( iter, runq )
     {
-        struct csched2_vcpu * iter_svc = __runq_elem(iter);
+        struct csched2_vcpu * iter_svc = runq_elem(iter);
 
         if ( svc->credit > iter_svc->credit )
             break;
 
         pos++;
     }
-
     list_add_tail(&svc->runq_elem, iter);
 
-    return pos;
-}
-
-static void
-runq_insert(const struct scheduler *ops, struct csched2_vcpu *svc)
-{
-    unsigned int cpu = svc->vcpu->processor;
-    struct list_head * runq = &c2rqd(ops, cpu)->runq;
-    int pos = 0;
-
-    ASSERT(spin_is_locked(per_cpu(schedule_data, cpu).schedule_lock));
-
-    ASSERT(!__vcpu_on_runq(svc));
-    ASSERT(c2r(ops, cpu) == c2r(ops, svc->vcpu->processor));
-
-    pos = __runq_insert(runq, svc);
-
     if ( unlikely(tb_init_done) )
     {
         struct {
@@ -923,14 +910,11 @@ runq_insert(const struct scheduler *ops, struct csched2_vcpu *svc)
                     sizeof(d),
                     (unsigned char *)&d);
     }
-
-    return;
 }
 
-static inline void
-__runq_remove(struct csched2_vcpu *svc)
+static inline void runq_remove(struct csched2_vcpu *svc)
 {
-    ASSERT(__vcpu_on_runq(svc));
+    ASSERT(vcpu_on_runq(svc));
     list_del_init(&svc->runq_elem);
 }
 
@@ -1281,8 +1265,8 @@ static void update_max_weight(struct csched2_runqueue_data *rqd, int new_weight,
 }
 
 #ifndef NDEBUG
-static /*inline*/ void
-__csched2_vcpu_check(struct vcpu *vc)
+static inline void
+csched2_vcpu_check(struct vcpu *vc)
 {
     struct csched2_vcpu * const svc = csched2_vcpu(vc);
     struct csched2_dom * const sdom = svc->sdom;
@@ -1300,7 +1284,7 @@ __csched2_vcpu_check(struct vcpu *vc)
     }
     SCHED_STAT_CRANK(vcpu_check);
 }
-#define CSCHED2_VCPU_CHECK(_vc)  (__csched2_vcpu_check(_vc))
+#define CSCHED2_VCPU_CHECK(_vc)  (csched2_vcpu_check(_vc))
 #else
 #define CSCHED2_VCPU_CHECK(_vc)
 #endif
@@ -1346,7 +1330,7 @@ csched2_alloc_vdata(const struct scheduler *ops, struct vcpu *vc, void *dd)
 
 /* Add and remove from runqueue assignment (not active run queue) */
 static void
-__runq_assign(struct csched2_vcpu *svc, struct csched2_runqueue_data *rqd)
+_runq_assign(struct csched2_vcpu *svc, struct csched2_runqueue_data *rqd)
 {
 
     svc->rqd = rqd;
@@ -1380,15 +1364,15 @@ runq_assign(const struct scheduler *ops, struct vcpu *vc)
 
     ASSERT(svc->rqd == NULL);
 
-    __runq_assign(svc, c2rqd(ops, vc->processor));
+    _runq_assign(svc, c2rqd(ops, vc->processor));
 }
 
 static void
-__runq_deassign(struct csched2_vcpu *svc)
+_runq_deassign(struct csched2_vcpu *svc)
 {
     struct csched2_runqueue_data *rqd = svc->rqd;
 
-    ASSERT(!__vcpu_on_runq(svc));
+    ASSERT(!vcpu_on_runq(svc));
     ASSERT(!(svc->flags & CSFLAG_scheduled));
 
     list_del_init(&svc->rqd_elem);
@@ -1407,7 +1391,7 @@ runq_deassign(const struct scheduler *ops, struct vcpu *vc)
 
     ASSERT(svc->rqd == c2rqd(ops, vc->processor));
 
-    __runq_deassign(svc);
+    _runq_deassign(svc);
 }
 
 static void
@@ -1420,11 +1404,11 @@ csched2_vcpu_sleep(const struct scheduler *ops, struct vcpu *vc)
 
     if ( curr_on_cpu(vc->processor) == vc )
         cpu_raise_softirq(vc->processor, SCHEDULE_SOFTIRQ);
-    else if ( __vcpu_on_runq(svc) )
+    else if ( vcpu_on_runq(svc) )
     {
         ASSERT(svc->rqd == c2rqd(ops, vc->processor));
         update_load(ops, svc->rqd, svc, -1, NOW());
-        __runq_remove(svc);
+        runq_remove(svc);
     }
     else if ( svc->flags & CSFLAG_delayed_runq_add )
         __clear_bit(__CSFLAG_delayed_runq_add, &svc->flags);
@@ -1447,7 +1431,7 @@ csched2_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
         goto out;
     }
 
-    if ( unlikely(__vcpu_on_runq(svc)) )
+    if ( unlikely(vcpu_on_runq(svc)) )
     {
         SCHED_STAT_CRANK(vcpu_wake_onrunq);
         goto out;
@@ -1509,7 +1493,7 @@ csched2_context_saved(const struct scheduler *ops, struct vcpu *vc)
     /* If someone wants it on the runqueue, put it there. */
     /*
      * NB: We can get rid of CSFLAG_scheduled by checking for
-     * vc->is_running and __vcpu_on_runq(svc) here.  However,
+     * vc->is_running and vcpu_on_runq(svc) here.  However,
      * since we're accessing the flags cacheline anyway,
      * it seems a bit pointless; especially as we have plenty of
      * bits free.
@@ -1517,7 +1501,7 @@ csched2_context_saved(const struct scheduler *ops, struct vcpu *vc)
     if ( __test_and_clear_bit(__CSFLAG_delayed_runq_add, &svc->flags)
          && likely(vcpu_runnable(vc)) )
     {
-        ASSERT(!__vcpu_on_runq(svc));
+        ASSERT(!vcpu_on_runq(svc));
 
         runq_insert(ops, svc);
         runq_tickle(ops, svc, now);
@@ -1749,13 +1733,13 @@ static void migrate(const struct scheduler *ops,
     {
         int on_runq = 0;
         /* It's not running; just move it */
-        if ( __vcpu_on_runq(svc) )
+        if ( vcpu_on_runq(svc) )
         {
-            __runq_remove(svc);
+            runq_remove(svc);
             update_load(ops, svc->rqd, NULL, -1, now);
             on_runq = 1;
         }
-        __runq_deassign(svc);
+        _runq_deassign(svc);
 
         cpumask_and(cpumask_scratch_cpu(cpu), svc->vcpu->cpu_hard_affinity,
                     cpupool_domain_cpumask(svc->vcpu->domain));
@@ -1764,7 +1748,7 @@ static void migrate(const struct scheduler *ops,
         svc->vcpu->processor = cpumask_any(cpumask_scratch_cpu(cpu));
         ASSERT(svc->vcpu->processor < nr_cpu_ids);
 
-        __runq_assign(svc, trqd);
+        _runq_assign(svc, trqd);
         if ( on_runq )
         {
             update_load(ops, svc->rqd, NULL, 1, now);
@@ -1814,7 +1798,7 @@ static void balance_load(const struct scheduler *ops, int cpu, s_time_t now)
     ASSERT(spin_is_locked(per_cpu(schedule_data, cpu).schedule_lock));
     st.lrqd = c2rqd(ops, cpu);
 
-    __update_runq_load(ops, st.lrqd, 0, now);
+    update_runq_load(ops, st.lrqd, 0, now);
 
 retry:
     if ( !read_trylock(&prv->lock) )
@@ -1832,7 +1816,7 @@ retry:
              || !spin_trylock(&st.orqd->lock) )
             continue;
 
-        __update_runq_load(ops, st.orqd, 0, now);
+        update_runq_load(ops, st.orqd, 0, now);
     
         delta = st.lrqd->b_avgload - st.orqd->b_avgload;
         if ( delta < 0 )
@@ -1931,7 +1915,7 @@ retry:
     {
         struct csched2_vcpu * push_svc = list_entry(push_iter, struct csched2_vcpu, rqd_elem);
 
-        __update_svc_load(ops, push_svc, 0, now);
+        update_svc_load(ops, push_svc, 0, now);
 
         if ( !vcpu_is_migrateable(push_svc, st.orqd) )
             continue;
@@ -1941,7 +1925,7 @@ retry:
             struct csched2_vcpu * pull_svc = list_entry(pull_iter, struct csched2_vcpu, rqd_elem);
             
             if ( !inner_load_updated )
-                __update_svc_load(ops, pull_svc, 0, now);
+                update_svc_load(ops, pull_svc, 0, now);
         
             if ( !vcpu_is_migrateable(pull_svc, st.lrqd) )
                 continue;
@@ -2009,12 +1993,12 @@ csched2_vcpu_migrate(
     if ( unlikely(!cpumask_test_cpu(new_cpu, cpupool_domain_cpumask(d))) )
     {
         ASSERT(system_state == SYS_STATE_suspend);
-        if ( __vcpu_on_runq(svc) )
+        if ( vcpu_on_runq(svc) )
         {
-            __runq_remove(svc);
+            runq_remove(svc);
             update_load(ops, svc->rqd, NULL, -1, now);
         }
-        __runq_deassign(svc);
+        _runq_deassign(svc);
         vc->processor = new_cpu;
         return;
     }
@@ -2302,7 +2286,7 @@ csched2_runtime(const struct scheduler *ops, int cpu,
      * run until your credit ~= his */
     if ( ! list_empty(runq) )
     {
-        struct csched2_vcpu *swait = __runq_elem(runq->next);
+        struct csched2_vcpu *swait = runq_elem(runq->next);
 
         if ( ! is_idle_vcpu(swait->vcpu)
              && swait->credit > 0 )
@@ -2566,7 +2550,7 @@ csched2_schedule(
             ASSERT(snext->rqd == rqd);
             ASSERT(!snext->vcpu->is_running);
 
-            __runq_remove(snext);
+            runq_remove(snext);
             __set_bit(__CSFLAG_scheduled, &snext->flags);
         }
 
@@ -2701,7 +2685,7 @@ csched2_dump_pcpu(const struct scheduler *ops, int cpu)
     loop = 0;
     list_for_each( iter, runq )
     {
-        svc = __runq_elem(iter);
+        svc = runq_elem(iter);
         if ( svc )
         {
             printk("\t%3d: ", ++loop);


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/9] xen: credit2: group the runq manipulating functions.
  2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
                   ` (2 preceding siblings ...)
  2017-01-26  0:30 ` [PATCH 3/9] xen: credit2: tidy up functions names by removing leading '__' Dario Faggioli
@ 2017-01-26  0:30 ` Dario Faggioli
  2017-01-26  0:30 ` [PATCH 5/9] xen: credit2: always mark a tickled pCPU as... tickled! Dario Faggioli
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:30 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Anshul Makkar

So that they're all close among each other, and
also near to the comment describind the runqueue
organization (which is also moved).

No functional change intended.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Anshul Makkar <anshul.makkar@citrix.com>
---
 xen/common/sched_credit2.c |  572 ++++++++++++++++++++++----------------------
 1 file changed, 286 insertions(+), 286 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index b764cf9..f4c0ae4 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -295,63 +295,6 @@ static int __read_mostly opt_overload_balance_tolerance = -3;
 integer_param("credit2_balance_over", opt_overload_balance_tolerance);
 
 /*
- * Runqueue organization.
- *
- * The various cpus are to be assigned each one to a runqueue, and we
- * want that to happen basing on topology. At the moment, it is possible
- * to choose to arrange runqueues to be:
- *
- * - per-core: meaning that there will be one runqueue per each physical
- *             core of the host. This will happen if the opt_runqueue
- *             parameter is set to 'core';
- *
- * - per-socket: meaning that there will be one runqueue per each physical
- *               socket (AKA package, which often, but not always, also
- *               matches a NUMA node) of the host; This will happen if
- *               the opt_runqueue parameter is set to 'socket';
- *
- * - per-node: meaning that there will be one runqueue per each physical
- *             NUMA node of the host. This will happen if the opt_runqueue
- *             parameter is set to 'node';
- *
- * - global: meaning that there will be only one runqueue to which all the
- *           (logical) processors of the host belong. This will happen if
- *           the opt_runqueue parameter is set to 'all'.
- *
- * Depending on the value of opt_runqueue, therefore, cpus that are part of
- * either the same physical core, the same physical socket, the same NUMA
- * node, or just all of them, will be put together to form runqueues.
- */
-#define OPT_RUNQUEUE_CORE   0
-#define OPT_RUNQUEUE_SOCKET 1
-#define OPT_RUNQUEUE_NODE   2
-#define OPT_RUNQUEUE_ALL    3
-static const char *const opt_runqueue_str[] = {
-    [OPT_RUNQUEUE_CORE] = "core",
-    [OPT_RUNQUEUE_SOCKET] = "socket",
-    [OPT_RUNQUEUE_NODE] = "node",
-    [OPT_RUNQUEUE_ALL] = "all"
-};
-static int __read_mostly opt_runqueue = OPT_RUNQUEUE_SOCKET;
-
-static void parse_credit2_runqueue(const char *s)
-{
-    unsigned int i;
-
-    for ( i = 0; i < ARRAY_SIZE(opt_runqueue_str); i++ )
-    {
-        if ( !strcmp(s, opt_runqueue_str[i]) )
-        {
-            opt_runqueue = i;
-            return;
-        }
-    }
-
-    printk("WARNING, unrecognized value of credit2_runqueue option!\n");
-}
-custom_param("credit2_runqueue", parse_credit2_runqueue);
-
-/*
  * Per-runqueue data
  */
 struct csched2_runqueue_data {
@@ -563,45 +506,304 @@ static int get_fallback_cpu(struct csched2_vcpu *svc)
         return cpumask_first(cpumask_scratch_cpu(cpu));
     }
 
-    ASSERT(!cpumask_empty(cpumask_scratch_cpu(cpu)));
-
-    return cpumask_first(cpumask_scratch_cpu(cpu));
+    ASSERT(!cpumask_empty(cpumask_scratch_cpu(cpu)));
+
+    return cpumask_first(cpumask_scratch_cpu(cpu));
+}
+
+/*
+ * Time-to-credit, credit-to-time.
+ *
+ * We keep track of the "residual" time to make sure that frequent short
+ * schedules still get accounted for in the end.
+ *
+ * FIXME: Do pre-calculated division?
+ */
+static void t2c_update(struct csched2_runqueue_data *rqd, s_time_t time,
+                          struct csched2_vcpu *svc)
+{
+    uint64_t val = time * rqd->max_weight + svc->residual;
+
+    svc->residual = do_div(val, svc->weight);
+    svc->credit -= val;
+}
+
+static s_time_t c2t(struct csched2_runqueue_data *rqd, s_time_t credit, struct csched2_vcpu *svc)
+{
+    return credit * svc->weight / rqd->max_weight;
+}
+
+/*
+ * Runqueue related code.
+ */
+
+/*
+ * Runqueue organization.
+ *
+ * The various cpus are to be assigned each one to a runqueue, and we
+ * want that to happen basing on topology. At the moment, it is possible
+ * to choose to arrange runqueues to be:
+ *
+ * - per-core: meaning that there will be one runqueue per each physical
+ *             core of the host. This will happen if the opt_runqueue
+ *             parameter is set to 'core';
+ *
+ * - per-socket: meaning that there will be one runqueue per each physical
+ *               socket (AKA package, which often, but not always, also
+ *               matches a NUMA node) of the host; This will happen if
+ *               the opt_runqueue parameter is set to 'socket';
+ *
+ * - per-node: meaning that there will be one runqueue per each physical
+ *             NUMA node of the host. This will happen if the opt_runqueue
+ *             parameter is set to 'node';
+ *
+ * - global: meaning that there will be only one runqueue to which all the
+ *           (logical) processors of the host belong. This will happen if
+ *           the opt_runqueue parameter is set to 'all'.
+ *
+ * Depending on the value of opt_runqueue, therefore, cpus that are part of
+ * either the same physical core, the same physical socket, the same NUMA
+ * node, or just all of them, will be put together to form runqueues.
+ */
+#define OPT_RUNQUEUE_CORE   0
+#define OPT_RUNQUEUE_SOCKET 1
+#define OPT_RUNQUEUE_NODE   2
+#define OPT_RUNQUEUE_ALL    3
+static const char *const opt_runqueue_str[] = {
+    [OPT_RUNQUEUE_CORE] = "core",
+    [OPT_RUNQUEUE_SOCKET] = "socket",
+    [OPT_RUNQUEUE_NODE] = "node",
+    [OPT_RUNQUEUE_ALL] = "all"
+};
+static int __read_mostly opt_runqueue = OPT_RUNQUEUE_SOCKET;
+
+static void parse_credit2_runqueue(const char *s)
+{
+    unsigned int i;
+
+    for ( i = 0; i < ARRAY_SIZE(opt_runqueue_str); i++ )
+    {
+        if ( !strcmp(s, opt_runqueue_str[i]) )
+        {
+            opt_runqueue = i;
+            return;
+        }
+    }
+
+    printk("WARNING, unrecognized value of credit2_runqueue option!\n");
+}
+custom_param("credit2_runqueue", parse_credit2_runqueue);
+
+static inline int vcpu_on_runq(struct csched2_vcpu *svc)
+{
+    return !list_empty(&svc->runq_elem);
+}
+
+static struct csched2_vcpu * runq_elem(struct list_head *elem)
+{
+    return list_entry(elem, struct csched2_vcpu, runq_elem);
+}
+
+static void activate_runqueue(struct csched2_private *prv, int rqi)
+{
+    struct csched2_runqueue_data *rqd;
+
+    rqd = prv->rqd + rqi;
+
+    BUG_ON(!cpumask_empty(&rqd->active));
+
+    rqd->max_weight = 1;
+    rqd->id = rqi;
+    INIT_LIST_HEAD(&rqd->svc);
+    INIT_LIST_HEAD(&rqd->runq);
+    spin_lock_init(&rqd->lock);
+
+    __cpumask_set_cpu(rqi, &prv->active_queues);
+}
+
+static void deactivate_runqueue(struct csched2_private *prv, int rqi)
+{
+    struct csched2_runqueue_data *rqd;
+
+    rqd = prv->rqd + rqi;
+
+    BUG_ON(!cpumask_empty(&rqd->active));
+
+    rqd->id = -1;
+
+    __cpumask_clear_cpu(rqi, &prv->active_queues);
+}
+
+static inline bool_t same_node(unsigned int cpua, unsigned int cpub)
+{
+    return cpu_to_node(cpua) == cpu_to_node(cpub);
+}
+
+static inline bool_t same_socket(unsigned int cpua, unsigned int cpub)
+{
+    return cpu_to_socket(cpua) == cpu_to_socket(cpub);
+}
+
+static inline bool_t same_core(unsigned int cpua, unsigned int cpub)
+{
+    return same_socket(cpua, cpub) &&
+           cpu_to_core(cpua) == cpu_to_core(cpub);
+}
+
+static unsigned int
+cpu_to_runqueue(struct csched2_private *prv, unsigned int cpu)
+{
+    struct csched2_runqueue_data *rqd;
+    unsigned int rqi;
+
+    for ( rqi = 0; rqi < nr_cpu_ids; rqi++ )
+    {
+        unsigned int peer_cpu;
+
+        /*
+         * As soon as we come across an uninitialized runqueue, use it.
+         * In fact, either:
+         *  - we are initializing the first cpu, and we assign it to
+         *    runqueue 0. This is handy, especially if we are dealing
+         *    with the boot cpu (if credit2 is the default scheduler),
+         *    as we would not be able to use cpu_to_socket() and similar
+         *    helpers anyway (they're result of which is not reliable yet);
+         *  - we have gone through all the active runqueues, and have not
+         *    found anyone whose cpus' topology matches the one we are
+         *    dealing with, so activating a new runqueue is what we want.
+         */
+        if ( prv->rqd[rqi].id == -1 )
+            break;
+
+        rqd = prv->rqd + rqi;
+        BUG_ON(cpumask_empty(&rqd->active));
+
+        peer_cpu = cpumask_first(&rqd->active);
+        BUG_ON(cpu_to_socket(cpu) == XEN_INVALID_SOCKET_ID ||
+               cpu_to_socket(peer_cpu) == XEN_INVALID_SOCKET_ID);
+
+        if ( opt_runqueue == OPT_RUNQUEUE_ALL ||
+             (opt_runqueue == OPT_RUNQUEUE_CORE && same_core(peer_cpu, cpu)) ||
+             (opt_runqueue == OPT_RUNQUEUE_SOCKET && same_socket(peer_cpu, cpu)) ||
+             (opt_runqueue == OPT_RUNQUEUE_NODE && same_node(peer_cpu, cpu)) )
+            break;
+    }
+
+    /* We really expect to be able to assign each cpu to a runqueue. */
+    BUG_ON(rqi >= nr_cpu_ids);
+
+    return rqi;
+}
+
+/* Find the domain with the highest weight. */
+static void update_max_weight(struct csched2_runqueue_data *rqd, int new_weight,
+                              int old_weight)
+{
+    /* Try to avoid brute-force search:
+     * - If new_weight is larger, max_weigth <- new_weight
+     * - If old_weight != max_weight, someone else is still max_weight
+     *   (No action required)
+     * - If old_weight == max_weight, brute-force search for max weight
+     */
+    if ( new_weight > rqd->max_weight )
+    {
+        rqd->max_weight = new_weight;
+        SCHED_STAT_CRANK(upd_max_weight_quick);
+    }
+    else if ( old_weight == rqd->max_weight )
+    {
+        struct list_head *iter;
+        int max_weight = 1;
+
+        list_for_each( iter, &rqd->svc )
+        {
+            struct csched2_vcpu * svc = list_entry(iter, struct csched2_vcpu, rqd_elem);
+
+            if ( svc->weight > max_weight )
+                max_weight = svc->weight;
+        }
+
+        rqd->max_weight = max_weight;
+        SCHED_STAT_CRANK(upd_max_weight_full);
+    }
+
+    if ( unlikely(tb_init_done) )
+    {
+        struct {
+            unsigned rqi:16, max_weight:16;
+        } d;
+        d.rqi = rqd->id;
+        d.max_weight = rqd->max_weight;
+        __trace_var(TRC_CSCHED2_RUNQ_MAX_WEIGHT, 1,
+                    sizeof(d),
+                    (unsigned char *)&d);
+    }
+}
+
+/* Add and remove from runqueue assignment (not active run queue) */
+static void
+_runq_assign(struct csched2_vcpu *svc, struct csched2_runqueue_data *rqd)
+{
+
+    svc->rqd = rqd;
+    list_add_tail(&svc->rqd_elem, &svc->rqd->svc);
+
+    update_max_weight(svc->rqd, svc->weight, 0);
+
+    /* Expected new load based on adding this vcpu */
+    rqd->b_avgload += svc->avgload;
+
+    if ( unlikely(tb_init_done) )
+    {
+        struct {
+            unsigned vcpu:16, dom:16;
+            unsigned rqi:16;
+        } d;
+        d.dom = svc->vcpu->domain->domain_id;
+        d.vcpu = svc->vcpu->vcpu_id;
+        d.rqi=rqd->id;
+        __trace_var(TRC_CSCHED2_RUNQ_ASSIGN, 1,
+                    sizeof(d),
+                    (unsigned char *)&d);
+    }
+
 }
 
-/*
- * Time-to-credit, credit-to-time.
- * 
- * We keep track of the "residual" time to make sure that frequent short
- * schedules still get accounted for in the end.
- *
- * FIXME: Do pre-calculated division?
- */
-static void t2c_update(struct csched2_runqueue_data *rqd, s_time_t time,
-                          struct csched2_vcpu *svc)
+static void
+runq_assign(const struct scheduler *ops, struct vcpu *vc)
 {
-    uint64_t val = time * rqd->max_weight + svc->residual;
+    struct csched2_vcpu *svc = vc->sched_priv;
 
-    svc->residual = do_div(val, svc->weight);
-    svc->credit -= val;
+    ASSERT(svc->rqd == NULL);
+
+    _runq_assign(svc, c2rqd(ops, vc->processor));
 }
 
-static s_time_t c2t(struct csched2_runqueue_data *rqd, s_time_t credit, struct csched2_vcpu *svc)
+static void
+_runq_deassign(struct csched2_vcpu *svc)
 {
-    return credit * svc->weight / rqd->max_weight;
-}
+    struct csched2_runqueue_data *rqd = svc->rqd;
 
-/*
- * Runqueue related code
- */
+    ASSERT(!vcpu_on_runq(svc));
+    ASSERT(!(svc->flags & CSFLAG_scheduled));
 
-static inline int vcpu_on_runq(struct csched2_vcpu *svc)
-{
-    return !list_empty(&svc->runq_elem);
+    list_del_init(&svc->rqd_elem);
+    update_max_weight(rqd, 0, svc->weight);
+
+    /* Expected new load based on removing this vcpu */
+    rqd->b_avgload = max_t(s_time_t, rqd->b_avgload - svc->avgload, 0);
+
+    svc->rqd = NULL;
 }
 
-static struct csched2_vcpu * runq_elem(struct list_head *elem)
+static void
+runq_deassign(const struct scheduler *ops, struct vcpu *vc)
 {
-    return list_entry(elem, struct csched2_vcpu, runq_elem);
+    struct csched2_vcpu *svc = vc->sched_priv;
+
+    ASSERT(svc->rqd == c2rqd(ops, vc->processor));
+
+    _runq_deassign(svc);
 }
 
 /*
@@ -1219,51 +1421,6 @@ void burn_credits(struct csched2_runqueue_data *rqd,
     }
 }
 
-/* Find the domain with the highest weight. */
-static void update_max_weight(struct csched2_runqueue_data *rqd, int new_weight,
-                              int old_weight)
-{
-    /* Try to avoid brute-force search:
-     * - If new_weight is larger, max_weigth <- new_weight
-     * - If old_weight != max_weight, someone else is still max_weight
-     *   (No action required)
-     * - If old_weight == max_weight, brute-force search for max weight
-     */
-    if ( new_weight > rqd->max_weight )
-    {
-        rqd->max_weight = new_weight;
-        SCHED_STAT_CRANK(upd_max_weight_quick);
-    }
-    else if ( old_weight == rqd->max_weight )
-    {
-        struct list_head *iter;
-        int max_weight = 1;
-
-        list_for_each( iter, &rqd->svc )
-        {
-            struct csched2_vcpu * svc = list_entry(iter, struct csched2_vcpu, rqd_elem);
-
-            if ( svc->weight > max_weight )
-                max_weight = svc->weight;
-        }
-
-        rqd->max_weight = max_weight;
-        SCHED_STAT_CRANK(upd_max_weight_full);
-    }
-
-    if ( unlikely(tb_init_done) )
-    {
-        struct {
-            unsigned rqi:16, max_weight:16;
-        } d;
-        d.rqi = rqd->id;
-        d.max_weight = rqd->max_weight;
-        __trace_var(TRC_CSCHED2_RUNQ_MAX_WEIGHT, 1,
-                    sizeof(d),
-                    (unsigned char *)&d);
-    }
-}
-
 #ifndef NDEBUG
 static inline void
 csched2_vcpu_check(struct vcpu *vc)
@@ -1328,72 +1485,6 @@ csched2_alloc_vdata(const struct scheduler *ops, struct vcpu *vc, void *dd)
     return svc;
 }
 
-/* Add and remove from runqueue assignment (not active run queue) */
-static void
-_runq_assign(struct csched2_vcpu *svc, struct csched2_runqueue_data *rqd)
-{
-
-    svc->rqd = rqd;
-    list_add_tail(&svc->rqd_elem, &svc->rqd->svc);
-
-    update_max_weight(svc->rqd, svc->weight, 0);
-
-    /* Expected new load based on adding this vcpu */
-    rqd->b_avgload += svc->avgload;
-
-    if ( unlikely(tb_init_done) )
-    {
-        struct {
-            unsigned vcpu:16, dom:16;
-            unsigned rqi:16;
-        } d;
-        d.dom = svc->vcpu->domain->domain_id;
-        d.vcpu = svc->vcpu->vcpu_id;
-        d.rqi=rqd->id;
-        __trace_var(TRC_CSCHED2_RUNQ_ASSIGN, 1,
-                    sizeof(d),
-                    (unsigned char *)&d);
-    }
-
-}
-
-static void
-runq_assign(const struct scheduler *ops, struct vcpu *vc)
-{
-    struct csched2_vcpu *svc = vc->sched_priv;
-
-    ASSERT(svc->rqd == NULL);
-
-    _runq_assign(svc, c2rqd(ops, vc->processor));
-}
-
-static void
-_runq_deassign(struct csched2_vcpu *svc)
-{
-    struct csched2_runqueue_data *rqd = svc->rqd;
-
-    ASSERT(!vcpu_on_runq(svc));
-    ASSERT(!(svc->flags & CSFLAG_scheduled));
-
-    list_del_init(&svc->rqd_elem);
-    update_max_weight(rqd, 0, svc->weight);
-
-    /* Expected new load based on removing this vcpu */
-    rqd->b_avgload = max_t(s_time_t, rqd->b_avgload - svc->avgload, 0);
-
-    svc->rqd = NULL;
-}
-
-static void
-runq_deassign(const struct scheduler *ops, struct vcpu *vc)
-{
-    struct csched2_vcpu *svc = vc->sched_priv;
-
-    ASSERT(svc->rqd == c2rqd(ops, vc->processor));
-
-    _runq_deassign(svc);
-}
-
 static void
 csched2_vcpu_sleep(const struct scheduler *ops, struct vcpu *vc)
 {
@@ -2778,97 +2869,6 @@ csched2_dump(const struct scheduler *ops)
 #undef cpustr
 }
 
-static void activate_runqueue(struct csched2_private *prv, int rqi)
-{
-    struct csched2_runqueue_data *rqd;
-
-    rqd = prv->rqd + rqi;
-
-    BUG_ON(!cpumask_empty(&rqd->active));
-
-    rqd->max_weight = 1;
-    rqd->id = rqi;
-    INIT_LIST_HEAD(&rqd->svc);
-    INIT_LIST_HEAD(&rqd->runq);
-    spin_lock_init(&rqd->lock);
-
-    __cpumask_set_cpu(rqi, &prv->active_queues);
-}
-
-static void deactivate_runqueue(struct csched2_private *prv, int rqi)
-{
-    struct csched2_runqueue_data *rqd;
-
-    rqd = prv->rqd + rqi;
-
-    BUG_ON(!cpumask_empty(&rqd->active));
-    
-    rqd->id = -1;
-
-    __cpumask_clear_cpu(rqi, &prv->active_queues);
-}
-
-static inline bool_t same_node(unsigned int cpua, unsigned int cpub)
-{
-    return cpu_to_node(cpua) == cpu_to_node(cpub);
-}
-
-static inline bool_t same_socket(unsigned int cpua, unsigned int cpub)
-{
-    return cpu_to_socket(cpua) == cpu_to_socket(cpub);
-}
-
-static inline bool_t same_core(unsigned int cpua, unsigned int cpub)
-{
-    return same_socket(cpua, cpub) &&
-           cpu_to_core(cpua) == cpu_to_core(cpub);
-}
-
-static unsigned int
-cpu_to_runqueue(struct csched2_private *prv, unsigned int cpu)
-{
-    struct csched2_runqueue_data *rqd;
-    unsigned int rqi;
-
-    for ( rqi = 0; rqi < nr_cpu_ids; rqi++ )
-    {
-        unsigned int peer_cpu;
-
-        /*
-         * As soon as we come across an uninitialized runqueue, use it.
-         * In fact, either:
-         *  - we are initializing the first cpu, and we assign it to
-         *    runqueue 0. This is handy, especially if we are dealing
-         *    with the boot cpu (if credit2 is the default scheduler),
-         *    as we would not be able to use cpu_to_socket() and similar
-         *    helpers anyway (they're result of which is not reliable yet);
-         *  - we have gone through all the active runqueues, and have not
-         *    found anyone whose cpus' topology matches the one we are
-         *    dealing with, so activating a new runqueue is what we want.
-         */
-        if ( prv->rqd[rqi].id == -1 )
-            break;
-
-        rqd = prv->rqd + rqi;
-        BUG_ON(cpumask_empty(&rqd->active));
-
-        peer_cpu = cpumask_first(&rqd->active);
-        BUG_ON(cpu_to_socket(cpu) == XEN_INVALID_SOCKET_ID ||
-               cpu_to_socket(peer_cpu) == XEN_INVALID_SOCKET_ID);
-
-        if ( opt_runqueue == OPT_RUNQUEUE_ALL ||
-             (opt_runqueue == OPT_RUNQUEUE_CORE && same_core(peer_cpu, cpu)) ||
-             (opt_runqueue == OPT_RUNQUEUE_SOCKET && same_socket(peer_cpu, cpu)) ||
-             (opt_runqueue == OPT_RUNQUEUE_NODE && same_node(peer_cpu, cpu)) )
-            break;
-    }
-
-    /* We really expect to be able to assign each cpu to a runqueue. */
-    BUG_ON(rqi >= nr_cpu_ids);
-
-    return rqi;
-}
-
 /* Returns the ID of the runqueue the cpu is assigned to. */
 static unsigned
 init_pdata(struct csched2_private *prv, unsigned int cpu)


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/9] xen: credit2: always mark a tickled pCPU as... tickled!
  2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
                   ` (3 preceding siblings ...)
  2017-01-26  0:30 ` [PATCH 4/9] xen: credit2: group the runq manipulating functions Dario Faggioli
@ 2017-01-26  0:30 ` Dario Faggioli
  2017-01-26  0:30 ` [PATCH 6/9] xen: credit2: don't miss accounting while doing a credit reset Dario Faggioli
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:30 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Anshul Makkar

In fact, whether or not a pCPU has been tickled, and is
therefore about to re-schedule, is something we look at
and base decisions on in various places.

So, let's make sure that we do that basing on accurate
information.

While there, also tweak a little bit smt_idle_mask_clear()
(used for implementing SMT support), so that it only alter
the relevant cpumask when there is the actual need for this.
(This is only for reduced overhead, behavior remains the
same).

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Anshul Makkar <anshul.makkar@citrix.com>
---
 xen/common/sched_credit2.c |   26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index f4c0ae4..bab9667 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -470,12 +470,15 @@ void smt_idle_mask_set(unsigned int cpu, const cpumask_t *idlers,
 }
 
 /*
- * Clear the bits of all the siblings of cpu from mask.
+ * Clear the bits of all the siblings of cpu from mask (if necessary).
  */
 static inline
 void smt_idle_mask_clear(unsigned int cpu, cpumask_t *mask)
 {
-    cpumask_andnot(mask, mask, per_cpu(cpu_sibling_mask, cpu));
+    const cpumask_t *cpu_siblings = per_cpu(cpu_sibling_mask, cpu);
+
+    if ( cpumask_subset(cpu_siblings, mask) )
+        cpumask_andnot(mask, mask, per_cpu(cpu_sibling_mask, cpu));
 }
 
 /*
@@ -1122,6 +1125,14 @@ static inline void runq_remove(struct csched2_vcpu *svc)
 
 void burn_credits(struct csched2_runqueue_data *rqd, struct csched2_vcpu *, s_time_t);
 
+static inline void
+tickle_cpu(unsigned int cpu, struct csched2_runqueue_data *rqd)
+{
+    __cpumask_set_cpu(cpu, &rqd->tickled);
+    smt_idle_mask_clear(cpu, &rqd->smt_idle);
+    cpu_raise_softirq(cpu, SCHEDULE_SOFTIRQ);
+}
+
 /*
  * Check what processor it is best to 'wake', for picking up a vcpu that has
  * just been put (back) in the runqueue. Logic is as follows:
@@ -1289,9 +1300,8 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
                     sizeof(d),
                     (unsigned char *)&d);
     }
-    __cpumask_set_cpu(ipid, &rqd->tickled);
-    smt_idle_mask_clear(ipid, &rqd->smt_idle);
-    cpu_raise_softirq(ipid, SCHEDULE_SOFTIRQ);
+
+    tickle_cpu(cpu, rqd);
 
     if ( unlikely(new->tickled_cpu != -1) )
         SCHED_STAT_CRANK(tickled_cpu_overwritten);
@@ -1494,7 +1504,9 @@ csched2_vcpu_sleep(const struct scheduler *ops, struct vcpu *vc)
     SCHED_STAT_CRANK(vcpu_sleep);
 
     if ( curr_on_cpu(vc->processor) == vc )
-        cpu_raise_softirq(vc->processor, SCHEDULE_SOFTIRQ);
+    {
+        tickle_cpu(vc->processor, svc->rqd);
+    }
     else if ( vcpu_on_runq(svc) )
     {
         ASSERT(svc->rqd == c2rqd(ops, vc->processor));
@@ -1817,8 +1829,8 @@ static void migrate(const struct scheduler *ops,
         svc->migrate_rqd = trqd;
         __set_bit(_VPF_migrating, &svc->vcpu->pause_flags);
         __set_bit(__CSFLAG_runq_migrate_request, &svc->flags);
-        cpu_raise_softirq(cpu, SCHEDULE_SOFTIRQ);
         SCHED_STAT_CRANK(migrate_requested);
+        tickle_cpu(cpu, svc->rqd);
     }
     else
     {


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/9] xen: credit2: don't miss accounting while doing a credit reset.
  2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
                   ` (4 preceding siblings ...)
  2017-01-26  0:30 ` [PATCH 5/9] xen: credit2: always mark a tickled pCPU as... tickled! Dario Faggioli
@ 2017-01-26  0:30 ` Dario Faggioli
  2017-01-26  0:30 ` [PATCH 7/9] xen/tools: tracing: credits can go negative, so use int Dario Faggioli
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:30 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Anshul Makkar

A credit reset basically means going through all the
vCPUs of a runqueue and altering their credits, as a
consequence of a 'scheduling epoch' having come to an
end.

Blocked or runnable vCPUs are fine, all the credits
they've spent running so far have been accounted to
them when they were scheduled out.

But if a vCPU is running on a pCPU, when a reset event
occurs (on another pCPU), that does not get properly
accounted. Let's therefore begin to do so, for better
accuracy and fairness.

In fact, after this patch, we see this in a trace:

 csched2:schedule cpu 10, rq# 1, busy, not tickled
 csched2:burn_credits d1v5, credit = 9998353, delta = 202996
 runstate_continue d1v5 running->running
 ...
 csched2:schedule cpu 12, rq# 1, busy, not tickled
 csched2:burn_credits d1v6, credit = -1327, delta = 9999544
 csched2:reset_credits d0v13, credit_start = 10500000, credit_end = 10500000, mult = 1
 csched2:reset_credits d0v14, credit_start = 10500000, credit_end = 10500000, mult = 1
 csched2:reset_credits d0v7, credit_start = 10500000, credit_end = 10500000, mult = 1
 csched2:burn_credits d1v5, credit = 201805, delta = 9796548
 csched2:reset_credits d1v5, credit_start = 201805, credit_end = 10201805, mult = 1
 csched2:burn_credits d1v6, credit = -1327, delta = 0
 csched2:reset_credits d1v6, credit_start = -1327, credit_end = 9998673, mult = 1

Which shows how d1v5 actually executed for ~9.796 ms,
on pCPU 10, when reset_credit() is executed, on pCPU
12, because of d1v6's credits going below 0.

Without this patch, this 9.796ms are not accounted
to anyone. With this patch, d1v5 is charged for that,
and its credits drop down from 9796548 to 201805.

And this is important, as it means that it will
begin the new epoch with 10201805 credits, instead
of 10500000 (which he would have, before this patch).

Basically, we were forgetting one round of accounting
in epoch x, for the vCPUs that are running at the time
the epoch ends. And this meant favouring a little bit
these same vCPUs, in epoch x+1, providing them with
the chance of execute longer than their fair share.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Anshul Makkar <anshul.makkar@citrix.com>
---
 xen/common/sched_credit2.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index bab9667..07e0a6d 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1341,18 +1341,28 @@ static void reset_credit(const struct scheduler *ops, int cpu, s_time_t now,
 
     list_for_each( iter, &rqd->svc )
     {
+        unsigned int svc_cpu;
         struct csched2_vcpu * svc;
         int start_credit;
 
         svc = list_entry(iter, struct csched2_vcpu, rqd_elem);
+        svc_cpu = svc->vcpu->processor;
 
         ASSERT(!is_idle_vcpu(svc->vcpu));
         ASSERT(svc->rqd == rqd);
 
+        /*
+         * If svc is running, it is our responsibility to make sure, here,
+         * that the credit it has spent so far get accounted.
+         */
+        if ( svc->vcpu == curr_on_cpu(svc_cpu) )
+            burn_credits(rqd, svc, now);
+
         start_credit = svc->credit;
 
-        /* And add INIT * m, avoiding integer multiplication in the
-         * common case. */
+        /*
+         * Add INIT * m, avoiding integer multiplication in the common case.
+         */
         if ( likely(m==1) )
             svc->credit += CSCHED2_CREDIT_INIT;
         else


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/9] xen/tools: tracing: credits can go negative, so use int.
  2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
                   ` (5 preceding siblings ...)
  2017-01-26  0:30 ` [PATCH 6/9] xen: credit2: don't miss accounting while doing a credit reset Dario Faggioli
@ 2017-01-26  0:30 ` Dario Faggioli
  2017-01-26  0:30 ` [PATCH 8/9] xen/tools: tracing: trace (Credit2) runq traversal Dario Faggioli
  2017-01-26  0:30 ` [PATCH 9/9] xen/tools: tracing: always report how long next slice will be Dario Faggioli
  8 siblings, 0 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:30 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap

For Credit2, in both the trace records, inside Xen,
and in their parsing, in xenalyze.

In fact, it is a lot easier to figure out how much
negative credits have gone for a certain vCPU by
looking at a negative integer, rather than to an
overflowed unsigned.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
 tools/xentrace/xenalyze.c  |   20 ++++++++++----------
 xen/common/sched_credit2.c |   10 +++++-----
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index f006804..a90da20 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -7651,11 +7651,11 @@ void sched_process(struct pcpu_info *p)
         case TRC_SCHED_CLASS_EVT(CSCHED2, 3): /* CREDIT_BURN       */
             if(opt.dump_all) {
                 struct {
-                    unsigned int vcpuid:16, domid:16, credit;
-                    int delta;
+                    unsigned int vcpuid:16, domid:16;
+                    int credit, delta;
                 } *r = (typeof(r))ri->d;
 
-                printf(" %s csched2:burn_credits d%uv%u, credit = %u, delta = %d\n",
+                printf(" %s csched2:burn_credits d%uv%u, credit = %d, delta = %d\n",
                        ri->dump_header, r->domid, r->vcpuid,
                        r->credit, r->delta);
             }
@@ -7664,10 +7664,10 @@ void sched_process(struct pcpu_info *p)
             if(opt.dump_all) {
                 struct {
                     unsigned int vcpuid:16, domid:16;
-                    unsigned int credit;
+                    int credit;
                 } *r = (typeof(r))ri->d;
 
-                printf(" %s csched2:tickle_check d%uv%u, credit = %u\n",
+                printf(" %s csched2:tickle_check d%uv%u, credit = %d\n",
                        ri->dump_header, r->domid, r->vcpuid, r->credit);
             }
             break;
@@ -7685,12 +7685,12 @@ void sched_process(struct pcpu_info *p)
             if(opt.dump_all) {
                 struct {
                     unsigned int vcpuid:16, domid:16;
-                    unsigned int credit_start, credit_end;
+                    int credit_start, credit_end;
                     unsigned int multiplier;
                 } *r = (typeof(r))ri->d;
 
                 printf(" %s csched2:reset_credits d%uv%u, "
-                       "credit_start = %u, credit_end = %u, mult = %u\n",
+                       "credit_start = %d, credit_end = %d, mult = %u\n",
                        ri->dump_header, r->domid, r->vcpuid,
                        r->credit_start, r->credit_end, r->multiplier);
             }
@@ -7752,12 +7752,12 @@ void sched_process(struct pcpu_info *p)
         case TRC_SCHED_CLASS_EVT(CSCHED2, 13): /* TICKLE_NEW       */
             if (opt.dump_all) {
                 struct {
-                    unsigned vcpuid:16, domid:16;
-                    unsigned processor, credit;
+                    unsigned int vcpuid:16, domid:16, processor;
+                    int credit;
                 } *r = (typeof(r))ri->d;
 
                 printf(" %s csched2:runq_tickle_new d%uv%u, "
-                       "processor = %u, credit = %u\n",
+                       "processor = %u, credit = %d\n",
                        ri->dump_header, r->domid, r->vcpuid,
                        r->processor, r->credit);
             }
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 07e0a6d..43db669 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1165,7 +1165,8 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
     {
         struct {
             unsigned vcpu:16, dom:16;
-            unsigned processor, credit;
+            unsigned processor;
+            int credit;
         } d;
         d.dom = new->vcpu->domain->domain_id;
         d.vcpu = new->vcpu->vcpu_id;
@@ -1264,7 +1265,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
         {
             struct {
                 unsigned vcpu:16, dom:16;
-                unsigned credit;
+                int credit;
             } d;
             d.dom = cur->vcpu->domain->domain_id;
             d.vcpu = cur->vcpu->vcpu_id;
@@ -1378,7 +1379,7 @@ static void reset_credit(const struct scheduler *ops, int cpu, s_time_t now,
         {
             struct {
                 unsigned vcpu:16, dom:16;
-                unsigned credit_start, credit_end;
+                int credit_start, credit_end;
                 unsigned multiplier;
             } d;
             d.dom = svc->vcpu->domain->domain_id;
@@ -1428,8 +1429,7 @@ void burn_credits(struct csched2_runqueue_data *rqd,
     {
         struct {
             unsigned vcpu:16, dom:16;
-            unsigned credit;
-            int delta;
+            int credit, delta;
         } d;
         d.dom = svc->vcpu->domain->domain_id;
         d.vcpu = svc->vcpu->vcpu_id;


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/9] xen/tools: tracing: trace (Credit2) runq traversal.
  2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
                   ` (6 preceding siblings ...)
  2017-01-26  0:30 ` [PATCH 7/9] xen/tools: tracing: credits can go negative, so use int Dario Faggioli
@ 2017-01-26  0:30 ` Dario Faggioli
  2017-01-26  0:30 ` [PATCH 9/9] xen/tools: tracing: always report how long next slice will be Dario Faggioli
  8 siblings, 0 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:30 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap

When traversing a Credit2 runqueue to select the
best candidate vCPU to be run next, show in the
trace which vCPUs we consider.

A bit verbose, but quite useful, considering that
we may end up looking at, but then discarding, one
of more vCPU. This will help understand which ones
are skipped and why.

Also, add how much credits the chosen vCPU has
(in the TRC_CSCHED2_RUNQ_CANDIDATE record). And,
while there, fix a bug in tools/xentrace/formats
(still in the output of TRC_CSCHED2_RUNQ_CANDIDATE).

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
 tools/xentrace/formats     |    3 ++-
 tools/xentrace/xenalyze.c  |   15 +++++++++++++--
 xen/common/sched_credit2.c |   15 +++++++++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/tools/xentrace/formats b/tools/xentrace/formats
index db89f92..72c0b24 100644
--- a/tools/xentrace/formats
+++ b/tools/xentrace/formats
@@ -65,9 +65,10 @@
 0x00022210  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched2:load_check     [ lrq_id[16]:orq_id[16] = 0x%(1)08x, delta = %(2)d ]
 0x00022211  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched2:load_balance   [ l_bavgload = 0x%(2)08x%(1)08x, o_bavgload = 0x%(4)08x%(3)08x, lrq_id[16]:orq_id[16] = 0x%(5)08x ]
 0x00022212  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched2:pick_cpu       [ b_avgload = 0x%(2)08x%(1)08x, dom:vcpu = 0x%(3)08x, rq_id[16]:new_cpu[16] = %(4)d ]
-0x00022213  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched2:runq_candidate [ dom:vcpu = 0x%(1)08x, skipped_vcpus = %(2)d tickled_cpu = %(3)d ]
+0x00022213  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched2:runq_candidate [ dom:vcpu = 0x%(1)08x, credit = %(4)d, skipped_vcpus = %(3)d, tickled_cpu = %(2)d ]
 0x00022214  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched2:schedule       [ rq:cpu = 0x%(1)08x, tasklet[8]:idle[8]:smt_idle[8]:tickled[8] = %(2)08x ]
 0x00022215  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched2:ratelimit      [ dom:vcpu = 0x%(1)08x, runtime = %(2)d ]
+0x00022216  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched2:runq_cand_chk  [ dom:vcpu = 0x%(1)08x ]
 
 0x00022801  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:tickle        [ cpu = %(1)d ]
 0x00022802  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtds:runq_pick     [ dom:vcpu = 0x%(1)08x, cur_deadline = 0x%(3)08x%(2)08x, cur_budget = 0x%(5)08x%(4)08x ]
diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index a90da20..2678e2a 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -7825,12 +7825,13 @@ void sched_process(struct pcpu_info *p)
                 struct {
                     unsigned vcpuid:16, domid:16;
                     unsigned tickled_cpu, skipped;
+                    int credit;
                 } *r = (typeof(r))ri->d;
 
-                printf(" %s csched2:runq_candidate d%uv%u, "
+                printf(" %s csched2:runq_candidate d%uv%u, credit = %d, "
                        "%u vcpus skipped, ",
                        ri->dump_header, r->domid, r->vcpuid,
-                       r->skipped);
+                       r->credit, r->skipped);
                 if (r->tickled_cpu == (unsigned)-1)
                     printf("no cpu was tickled\n");
                 else
@@ -7864,6 +7865,16 @@ void sched_process(struct pcpu_info *p)
                        r->runtime / 1000, r->runtime % 1000);
             }
             break;
+        case TRC_SCHED_CLASS_EVT(CSCHED2, 23): /* RUNQ_CAND_CHECK  */
+            if(opt.dump_all) {
+                struct {
+                    unsigned int vcpuid:16, domid:16;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s csched2:runq_cand_check d%uv%u\n",
+                       ri->dump_header, r->domid, r->vcpuid);
+            }
+            break;
         /* RTDS (TRC_RTDS_xxx) */
         case TRC_SCHED_CLASS_EVT(RTDS, 1): /* TICKLE           */
             if(opt.dump_all) {
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 43db669..81149cf 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -57,6 +57,7 @@
 #define TRC_CSCHED2_RUNQ_CANDIDATE   TRC_SCHED_CLASS_EVT(CSCHED2, 20)
 #define TRC_CSCHED2_SCHEDULE         TRC_SCHED_CLASS_EVT(CSCHED2, 21)
 #define TRC_CSCHED2_RATELIMIT        TRC_SCHED_CLASS_EVT(CSCHED2, 22)
+#define TRC_CSCHED2_RUNQ_CAND_CHECK  TRC_SCHED_CLASS_EVT(CSCHED2, 23)
 
 /*
  * WARNING: This is still in an experimental phase.  Status and work can be found at the
@@ -2497,6 +2498,18 @@ runq_candidate(struct csched2_runqueue_data *rqd,
     {
         struct csched2_vcpu * svc = list_entry(iter, struct csched2_vcpu, runq_elem);
 
+        if ( unlikely(tb_init_done) )
+        {
+            struct {
+                unsigned vcpu:16, dom:16;
+            } d;
+            d.dom = svc->vcpu->domain->domain_id;
+            d.vcpu = svc->vcpu->vcpu_id;
+            __trace_var(TRC_CSCHED2_RUNQ_CAND_CHECK, 1,
+                        sizeof(d),
+                        (unsigned char *)&d);
+        }
+
         /* Only consider vcpus that are allowed to run on this processor. */
         if ( !cpumask_test_cpu(cpu, svc->vcpu->cpu_hard_affinity) )
         {
@@ -2545,9 +2558,11 @@ runq_candidate(struct csched2_runqueue_data *rqd,
         struct {
             unsigned vcpu:16, dom:16;
             unsigned tickled_cpu, skipped;
+            int credit;
         } d;
         d.dom = snext->vcpu->domain->domain_id;
         d.vcpu = snext->vcpu->vcpu_id;
+        d.credit = snext->credit;
         d.tickled_cpu = snext->tickled_cpu;
         d.skipped = *skipped;
         __trace_var(TRC_CSCHED2_RUNQ_CANDIDATE, 1,


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 9/9] xen/tools: tracing: always report how long next slice will be.
  2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
                   ` (7 preceding siblings ...)
  2017-01-26  0:30 ` [PATCH 8/9] xen/tools: tracing: trace (Credit2) runq traversal Dario Faggioli
@ 2017-01-26  0:30 ` Dario Faggioli
  8 siblings, 0 replies; 10+ messages in thread
From: Dario Faggioli @ 2017-01-26  0:30 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap

In fact, it is quite useful a pice of information,
to know how long it is the "next time slice" a vCPU
has been granted. It allows one to assume (and then
check) when the scheduler is supposed to run next,
and other things.

While this information is indeed reported when a
context switch happens, it is not when the same vCPU
that is running, continues to run.

Fix that, with the following outcome.

Before this change:

 csched2:schedule cpu 9, rq# 1, idle, SMT idle, tickled
 csched2:runq_candidate d0v3, 0 vcpus skipped, cpu 9 was tickled
 sched_switch prev d32767v9, run for 991.186us
 sched_switch next d0v3, was runnable for 2.515us, next slice 10000.0us
 sched_switch prev d32767v9 next d0v3              ^^^^^^^^^^^^^^^^^^^^
 runstate_change d32767v9 running->runnable
 ...
 csched2:schedule cpu 2, rq# 0, busy, not tickled
 csched2:burn_credits d1v5, credit = 9996950, delta = 502913
 csched2:runq_candidate d1v5, 0 vcpus skipped, no cpu was tickled
 runstate_continue d1v5 running->running
                                         ?????????????

That is, in the second block, it is shown that d1v5
was already running and will continue to do so,
but we have no idea for how long.

OTOH, after the change:

 csched2:schedule cpu 1, rq# 0, busy, not tickled
 csched2:burn_credits d0v8, credit = 9998645, delta = 12104
 csched2:runq_candidate d0v8, credit = 9998645, 0 vcpus skipped, no cpu was tickled
 sched_switch continue d0v8, run for 1125.820us, next slice 9998.645us
 runstate_continue d0v8 running->running         ^^^^^^^^^^^^^^^^^^^^^

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
 tools/xentrace/formats     |    1 +
 tools/xentrace/xenalyze.c  |   17 +++++++++++++++++
 xen/common/schedule.c      |    4 ++++
 xen/include/public/trace.h |    1 +
 4 files changed, 23 insertions(+)

diff --git a/tools/xentrace/formats b/tools/xentrace/formats
index 72c0b24..a055231 100644
--- a/tools/xentrace/formats
+++ b/tools/xentrace/formats
@@ -35,6 +35,7 @@
 0x0002800e  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  switch_infprev    [ dom:vcpu = 0x%(1)04x%(2)04x, runtime = %(3)d ]
 0x0002800f  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  switch_infnext    [ new_dom:vcpu = 0x%(1)04x%(2)04x, time = %(3)d, r_time = %(4)d ]
 0x00028010  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  domain_shutdown_code [ dom:vcpu = 0x%(1)04x%(2)04x, reason = 0x%(3)08x ]
+0x00028011  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  switch_infcont    [ dom:vcpu = 0x%(1)04x%(2)04x, runtime = %(3)d, r_time = %(4)d ]
 
 0x00022001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched:sched_tasklet
 0x00022002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  csched:account_start [ dom:vcpu = 0x%(1)04x%(2)04x, active = %(3)d ]
diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index 2678e2a..68ffcc2 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -7528,6 +7528,23 @@ void sched_process(struct pcpu_info *p)
                 printf("\n");
             }
             break;
+        case TRC_SCHED_SWITCH_INFCONT:
+            if(opt.dump_all)
+            {
+                struct {
+                    unsigned int domid, vcpuid, rsince;
+                    int slice;
+                } *r = (typeof(r))ri->d;
+
+                printf(" %s sched_switch continue d%uv%u, run for %u.%uus",
+                       ri->dump_header, r->domid, r->vcpuid,
+                       r->rsince / 1000, r->rsince % 1000);
+                if ( r->slice > 0 )
+                    printf(", next slice %u.%uus", r->slice / 1000,
+                           r->slice % 1000);
+                printf("\n");
+            }
+            break;
         case TRC_SCHED_CTL:
         case TRC_SCHED_S_TIMER_FN:
         case TRC_SCHED_T_TIMER_FN:
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 43b5b99..3947f6c 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1398,6 +1398,10 @@ static void schedule(void)
     if ( unlikely(prev == next) )
     {
         pcpu_schedule_unlock_irq(lock, cpu);
+        TRACE_4D(TRC_SCHED_SWITCH_INFCONT,
+                 next->domain->domain_id, next->vcpu_id,
+                 now - prev->runstate.state_entry_time,
+                 next_slice.time);
         trace_continue_running(next);
         return continue_running(prev);
     }
diff --git a/xen/include/public/trace.h b/xen/include/public/trace.h
index 5ef9c37..7f2e891 100644
--- a/xen/include/public/trace.h
+++ b/xen/include/public/trace.h
@@ -115,6 +115,7 @@
 #define TRC_SCHED_SWITCH_INFPREV (TRC_SCHED_VERBOSE + 14)
 #define TRC_SCHED_SWITCH_INFNEXT (TRC_SCHED_VERBOSE + 15)
 #define TRC_SCHED_SHUTDOWN_CODE  (TRC_SCHED_VERBOSE + 16)
+#define TRC_SCHED_SWITCH_INFCONT (TRC_SCHED_VERBOSE + 17)
 
 #define TRC_DOM0_DOM_ADD         (TRC_DOM0_DOMOPS + 1)
 #define TRC_DOM0_DOM_REM         (TRC_DOM0_DOMOPS + 2)


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-01-26  0:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-26  0:29 [PATCH 0/9] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
2017-01-26  0:29 ` [PATCH 1/9] xen: credit2: improve comments' style and definition of CSFLAG-s Dario Faggioli
2017-01-26  0:30 ` [PATCH 2/9] xen: credit2: make accessor helpers inline functions instead of macros Dario Faggioli
2017-01-26  0:30 ` [PATCH 3/9] xen: credit2: tidy up functions names by removing leading '__' Dario Faggioli
2017-01-26  0:30 ` [PATCH 4/9] xen: credit2: group the runq manipulating functions Dario Faggioli
2017-01-26  0:30 ` [PATCH 5/9] xen: credit2: always mark a tickled pCPU as... tickled! Dario Faggioli
2017-01-26  0:30 ` [PATCH 6/9] xen: credit2: don't miss accounting while doing a credit reset Dario Faggioli
2017-01-26  0:30 ` [PATCH 7/9] xen/tools: tracing: credits can go negative, so use int Dario Faggioli
2017-01-26  0:30 ` [PATCH 8/9] xen/tools: tracing: trace (Credit2) runq traversal Dario Faggioli
2017-01-26  0:30 ` [PATCH 9/9] xen/tools: tracing: always report how long next slice will be Dario Faggioli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.