* [PATCH v4 1/7] xen: credit2: always mark a tickled pCPU as... tickled!
2017-03-01 14:52 [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
@ 2017-03-01 14:52 ` Dario Faggioli
2017-03-01 14:52 ` [PATCH v4 2/7] xen: credit2: don't miss accounting while doing a credit reset Dario Faggioli
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Dario Faggioli @ 2017-03-01 14:52 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>
Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
---
Cc: Anshul Makkar <anshul.makkar@citrix.com>
---
Changes from v3:
* moved at the top of the series.
Changes from v2:
* fixed a bug I found myself in runq_tickle().
---
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 b12d038..cfb783d 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -504,12 +504,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));
}
/*
@@ -913,6 +916,14 @@ __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:
@@ -1080,9 +1091,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(ipid, rqd);
if ( unlikely(new->tickled_cpu != -1) )
SCHED_STAT_CRANK(tickled_cpu_overwritten);
@@ -1395,7 +1405,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 == RQD(ops, vc->processor));
@@ -1718,8 +1730,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] 13+ messages in thread* [PATCH v4 2/7] xen: credit2: don't miss accounting while doing a credit reset.
2017-03-01 14:52 [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
2017-03-01 14:52 ` [PATCH v4 1/7] xen: credit2: always mark a tickled pCPU as... tickled! Dario Faggioli
@ 2017-03-01 14:52 ` Dario Faggioli
2017-03-01 14:53 ` [PATCH v4 3/7] xen: credit2: make accessor helpers inline functions instead of macros Dario Faggioli
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Dario Faggioli @ 2017-03-01 14:52 UTC (permalink / raw)
To: xen-devel; +Cc: Anshul Makkar, George Dunlap
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>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---
Cc: Anshul Makkar <anshul.makkar@citrix.com>
---
Changes from v3:
* moved at top of the series.
---
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 cfb783d..d6b4a70 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1132,18 +1132,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] 13+ messages in thread* [PATCH v4 3/7] xen: credit2: make accessor helpers inline functions instead of macros
2017-03-01 14:52 [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
2017-03-01 14:52 ` [PATCH v4 1/7] xen: credit2: always mark a tickled pCPU as... tickled! Dario Faggioli
2017-03-01 14:52 ` [PATCH v4 2/7] xen: credit2: don't miss accounting while doing a credit reset Dario Faggioli
@ 2017-03-01 14:53 ` Dario Faggioli
2017-03-01 16:36 ` George Dunlap
2017-03-01 14:53 ` [PATCH v4 4/7] xen: credit2: tidy up functions names by removing leading '__' Dario Faggioli
` (4 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Dario Faggioli @ 2017-03-01 14:53 UTC (permalink / raw)
To: xen-devel; +Cc: George Dunlap, Andrew Cooper, Anshul Makkar, Jan Beulich
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>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
---
Changes from v2:
* plain 'inline' instead of 'always_inline', as requested during review;
* 'unsigned int' instead of just 'unisgned', as requested during review;
* constified more, as suggested during review;
* killed pointless parantheses, as suggested during review.
---
xen/common/sched_credit2.c | 153 +++++++++++++++++++++++++-------------------
1 file changed, 86 insertions(+), 67 deletions(-)
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index d6b4a70..1be4385 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -208,18 +208,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
@@ -440,6 +428,37 @@ struct csched2_dom {
};
/*
+ * Accessor helpers functions.
+ */
+static inline struct csched2_private *csched2_priv(const struct scheduler *ops)
+{
+ return ops->sched_data;
+}
+
+static inline struct csched2_vcpu *csched2_vcpu(const struct vcpu *v)
+{
+ return v->sched_priv;
+}
+
+static inline struct csched2_dom *csched2_dom(const struct domain *d)
+{
+ return d->sched_priv;
+}
+
+/* CPU to runq_id macro */
+static inline int c2r(const struct scheduler *ops, unsigned int cpu)
+{
+ return csched2_priv(ops)->runq_map[(cpu)];
+}
+
+/* CPU to runqueue struct macro */
+static inline struct csched2_runqueue_data *c2rqd(const struct scheduler *ops,
+ unsigned int 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
@@ -696,7 +715,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;
@@ -783,7 +802,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;
@@ -880,7 +899,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));
@@ -946,7 +965,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;
@@ -1017,7 +1036,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_and_clear_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 )
@@ -1033,7 +1052,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
/* Already looked at this one above */
ASSERT(i != cpu);
- 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
@@ -1105,7 +1124,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;
@@ -1193,7 +1212,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)) )
{
@@ -1280,11 +1299,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) );
@@ -1324,7 +1343,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
@@ -1376,7 +1395,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
@@ -1401,7 +1420,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);
}
@@ -1409,7 +1428,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);
@@ -1420,7 +1439,7 @@ csched2_vcpu_sleep(const struct scheduler *ops, struct vcpu *vc)
}
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);
}
@@ -1431,7 +1450,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;
@@ -1469,7 +1488,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();
@@ -1486,7 +1505,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);
}
@@ -1494,12 +1513,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);
@@ -1530,9 +1549,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));
@@ -1795,7 +1814,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;
@@ -1810,7 +1829,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);
@@ -1983,7 +2002,7 @@ 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();
@@ -2016,10 +2035,10 @@ csched2_vcpu_migrate(
}
/* If here, new_cpu must be a valid Credit2 pCPU, and in our affinity. */
- 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));
- trqd = RQD(ops, new_cpu);
+ trqd = c2rqd(ops, new_cpu);
/*
* Do the actual movement toward new_cpu, and update vc->processor.
@@ -2041,8 +2060,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;
@@ -2075,10 +2094,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);
@@ -2102,7 +2121,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 )
@@ -2133,7 +2152,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;
@@ -2149,7 +2168,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);
@@ -2178,7 +2197,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);
@@ -2192,9 +2211,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
@@ -2239,7 +2258,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));
@@ -2264,9 +2283,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)
@@ -2355,7 +2374,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;
@@ -2394,7 +2413,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 )
{
@@ -2474,7 +2493,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;
@@ -2483,9 +2502,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));
@@ -2543,7 +2562,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);
@@ -2664,7 +2683,7 @@ csched2_dump_vcpu(struct csched2_private *prv, struct csched2_vcpu *svc)
static inline void
dump_pcpu(const struct scheduler *ops, int cpu)
{
- struct csched2_private *prv = CSCHED2_PRIV(ops);
+ struct csched2_private *prv = csched2_priv(ops);
struct csched2_vcpu *svc;
#define cpustr keyhandler_scratch
@@ -2674,7 +2693,7 @@ dump_pcpu(const struct scheduler *ops, int cpu)
printk("core=%s\n", cpustr);
/* current VCPU (nothing to say if that's the idle vcpu) */
- svc = CSCHED2_VCPU(curr_on_cpu(cpu));
+ svc = csched2_vcpu(curr_on_cpu(cpu));
if ( svc && !is_idle_vcpu(svc->vcpu) )
{
printk("\trun: ");
@@ -2687,7 +2706,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;
unsigned int i, j, loop;
#define cpustr keyhandler_scratch
@@ -2747,7 +2766,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);
@@ -2918,7 +2937,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;
@@ -2946,7 +2965,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;
@@ -2993,7 +3012,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;
@@ -3102,7 +3121,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] 13+ messages in thread* [PATCH v4 4/7] xen: credit2: tidy up functions names by removing leading '__'.
2017-03-01 14:52 [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
` (2 preceding siblings ...)
2017-03-01 14:53 ` [PATCH v4 3/7] xen: credit2: make accessor helpers inline functions instead of macros Dario Faggioli
@ 2017-03-01 14:53 ` Dario Faggioli
2017-03-01 14:53 ` [PATCH v4 5/7] xen: credit2: group the runq manipulating functions Dario Faggioli
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Dario Faggioli @ 2017-03-01 14:53 UTC (permalink / raw)
To: xen-devel; +Cc: Anshul Makkar, George Dunlap
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_assign() and
__runq_deassign() (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>
Acked-by: George Dunlap <george.dunlap@citrix.com>
---
Cc: Anshul Makkar <anshul.makkar@citrix.com>
---
Changes from v2:
* made 'runq_elem()' inline as well, as suggested during review.
---
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 1be4385..1f57239 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -220,7 +220,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:
@@ -232,7 +232,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:
*
@@ -593,14 +593,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 inline struct csched2_vcpu * runq_elem(struct list_head *elem)
{
return list_entry(elem, struct csched2_vcpu, runq_elem);
}
@@ -712,8 +710,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;
@@ -799,8 +797,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;
@@ -864,17 +862,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);
@@ -882,33 +887,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 {
@@ -922,14 +909,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);
}
@@ -1296,8 +1280,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;
@@ -1315,7 +1299,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
@@ -1361,7 +1345,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;
@@ -1395,15 +1379,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);
@@ -1422,7 +1406,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
@@ -1437,11 +1421,11 @@ csched2_vcpu_sleep(const struct scheduler *ops, struct vcpu *vc)
{
tickle_cpu(vc->processor, svc->rqd);
}
- 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);
@@ -1464,7 +1448,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;
@@ -1526,7 +1510,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.
@@ -1534,7 +1518,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);
@@ -1766,13 +1750,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));
@@ -1781,7 +1765,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);
@@ -1831,7 +1815,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) )
@@ -1849,7 +1833,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 )
@@ -1950,7 +1934,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;
@@ -1960,7 +1944,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;
@@ -2024,12 +2008,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;
}
@@ -2321,7 +2305,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 )
@@ -2585,7 +2569,7 @@ csched2_schedule(
ASSERT(snext->rqd == rqd);
ASSERT(!snext->vcpu->is_running);
- __runq_remove(snext);
+ runq_remove(snext);
__set_bit(__CSFLAG_scheduled, &snext->flags);
}
@@ -2795,7 +2779,7 @@ csched2_dump(const struct scheduler *ops)
printk("RUNQ:\n");
list_for_each( iter, runq )
{
- struct csched2_vcpu *svc = __runq_elem(iter);
+ struct csched2_vcpu *svc = runq_elem(iter);
if ( svc )
{
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 5/7] xen: credit2: group the runq manipulating functions.
2017-03-01 14:52 [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
` (3 preceding siblings ...)
2017-03-01 14:53 ` [PATCH v4 4/7] xen: credit2: tidy up functions names by removing leading '__' Dario Faggioli
@ 2017-03-01 14:53 ` Dario Faggioli
2017-03-01 16:41 ` George Dunlap
2017-03-01 14:53 ` [PATCH v4 6/7] xen/tools: tracing: trace (Credit2) runq traversal Dario Faggioli
` (2 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Dario Faggioli @ 2017-03-01 14:53 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 describing 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>
---
Changes from v3:
* fix a typo in the changelog.
Changes from v2:
* don't move the 'credit2_runqueue' option parsing code, as suggested during
review;
---
xen/common/sched_credit2.c | 408 ++++++++++++++++++++++----------------------
1 file changed, 204 insertions(+), 204 deletions(-)
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 1f57239..66b7f96 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -569,7 +569,7 @@ static int get_fallback_cpu(struct csched2_vcpu *svc)
/*
* 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.
*
@@ -590,7 +590,7 @@ static s_time_t c2t(struct csched2_runqueue_data *rqd, s_time_t credit, struct c
}
/*
- * Runqueue related code
+ * Runqueue related code.
*/
static inline int vcpu_on_runq(struct csched2_vcpu *svc)
@@ -603,6 +603,208 @@ static inline 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);
+ }
+
+}
+
+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);
+}
+
/*
* Track the runq load by gathering instantaneous load samples, and using
* exponentially weighted moving average (EWMA) for the 'decaying'.
@@ -1234,51 +1436,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)
@@ -1343,72 +1500,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)
{
@@ -2794,97 +2885,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] 13+ messages in thread* Re: [PATCH v4 5/7] xen: credit2: group the runq manipulating functions.
2017-03-01 14:53 ` [PATCH v4 5/7] xen: credit2: group the runq manipulating functions Dario Faggioli
@ 2017-03-01 16:41 ` George Dunlap
2017-03-01 17:12 ` Dario Faggioli
0 siblings, 1 reply; 13+ messages in thread
From: George Dunlap @ 2017-03-01 16:41 UTC (permalink / raw)
To: Dario Faggioli, xen-devel; +Cc: George Dunlap, Anshul Makkar
On 01/03/17 14:53, Dario Faggioli wrote:
> So that they're all close among each other, and
> also near to the comment describing 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>
> ---
> Changes from v3:
> * fix a typo in the changelog.
>
> Changes from v2:
> * don't move the 'credit2_runqueue' option parsing code, as suggested during
> review;
> ---
> xen/common/sched_credit2.c | 408 ++++++++++++++++++++++----------------------
> 1 file changed, 204 insertions(+), 204 deletions(-)
>
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index 1f57239..66b7f96 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -569,7 +569,7 @@ static int get_fallback_cpu(struct csched2_vcpu *svc)
>
> /*
> * 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.
> *
> @@ -590,7 +590,7 @@ static s_time_t c2t(struct csched2_runqueue_data *rqd, s_time_t credit, struct c
> }
>
> /*
> - * Runqueue related code
> + * Runqueue related code.
You realize we changed CODING_STYLE to not require a full stop if there
was only one line? This isn't a complete sentence so I would leave the
full stop out.
But that's just for future reference -- I don't care enough about this
in general, I just wanted to raise it to your attention. :-)
Acked-by: George Dunlap <george.dunlap@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 5/7] xen: credit2: group the runq manipulating functions.
2017-03-01 16:41 ` George Dunlap
@ 2017-03-01 17:12 ` Dario Faggioli
0 siblings, 0 replies; 13+ messages in thread
From: Dario Faggioli @ 2017-03-01 17:12 UTC (permalink / raw)
To: George Dunlap, xen-devel; +Cc: George Dunlap, Anshul Makkar
[-- Attachment #1.1: Type: text/plain, Size: 1138 bytes --]
On Wed, 2017-03-01 at 16:41 +0000, George Dunlap wrote:
> On 01/03/17 14:53, Dario Faggioli wrote:
> > @@ -590,7 +590,7 @@ static s_time_t c2t(struct
> > csched2_runqueue_data *rqd, s_time_t credit, struct c
> > }
> >
> > /*
> > - * Runqueue related code
> > + * Runqueue related code.
>
> You realize we changed CODING_STYLE to not require a full stop if
> there
> was only one line? This isn't a complete sentence so I would leave
> the
> full stop out.
>
Now that you mention it, yes, I remember the thread. But my
understanding of the results of such thread was actually a bit
different from what you say and explain above.
So...
> But that's just for future reference -- I don't care enough about
> this
> in general, I just wanted to raise it to your attention. :-)
>
...that's been useful and much appreciated. Thanks!
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 6/7] xen/tools: tracing: trace (Credit2) runq traversal.
2017-03-01 14:52 [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
` (4 preceding siblings ...)
2017-03-01 14:53 ` [PATCH v4 5/7] xen: credit2: group the runq manipulating functions Dario Faggioli
@ 2017-03-01 14:53 ` Dario Faggioli
2017-03-01 14:53 ` [PATCH v4 7/7] xen/tools: tracing: Report next slice time when continuing as well as switching Dario Faggioli
2017-03-01 15:34 ` [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
7 siblings, 0 replies; 13+ messages in thread
From: Dario Faggioli @ 2017-03-01 14:53 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>
Acked-by: George Dunlap <george.dunlap@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 66b7f96..af457c1 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -56,6 +56,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
@@ -2494,6 +2495,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) )
{
@@ -2542,9 +2555,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] 13+ messages in thread* [PATCH v4 7/7] xen/tools: tracing: Report next slice time when continuing as well as switching
2017-03-01 14:52 [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
` (5 preceding siblings ...)
2017-03-01 14:53 ` [PATCH v4 6/7] xen/tools: tracing: trace (Credit2) runq traversal Dario Faggioli
@ 2017-03-01 14:53 ` Dario Faggioli
2017-03-01 16:53 ` George Dunlap
2017-03-01 15:34 ` [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
7 siblings, 1 reply; 13+ messages in thread
From: Dario Faggioli @ 2017-03-01 14:53 UTC (permalink / raw)
To: xen-devel; +Cc: George Dunlap
We record trace information about the next timeslice when
switching to a different vcpu, but not when continuing to
run the same cpu:
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
?????????????
This information is quite useful; so add a trace including
that information on the 'continue_running' path as well,
like this:
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>
---
Changes from v2:
* reworded the changelog, as suggested during review.
---
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 36bbd94..223a120 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -1397,6 +1397,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] 13+ messages in thread* Re: [PATCH v4 7/7] xen/tools: tracing: Report next slice time when continuing as well as switching
2017-03-01 14:53 ` [PATCH v4 7/7] xen/tools: tracing: Report next slice time when continuing as well as switching Dario Faggioli
@ 2017-03-01 16:53 ` George Dunlap
0 siblings, 0 replies; 13+ messages in thread
From: George Dunlap @ 2017-03-01 16:53 UTC (permalink / raw)
To: Dario Faggioli, xen-devel; +Cc: George Dunlap
On 01/03/17 14:53, Dario Faggioli wrote:
> We record trace information about the next timeslice when
> switching to a different vcpu, but not when continuing to
> run the same cpu:
>
> 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
> ?????????????
>
> This information is quite useful; so add a trace including
> that information on the 'continue_running' path as well,
> like this:
>
> 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>
That looks good, thanks:
Acked-by: George Dunlap <george.dunlap@citrix.com>
> ---
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> Changes from v2:
> * reworded the changelog, as suggested during review.
> ---
> 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 36bbd94..223a120 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1397,6 +1397,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 [flat|nested] 13+ messages in thread
* Re: [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs
2017-03-01 14:52 [PATCH v4 0/7] xen: credit2: improve style, and tracing; fix two bugs Dario Faggioli
` (6 preceding siblings ...)
2017-03-01 14:53 ` [PATCH v4 7/7] xen/tools: tracing: Report next slice time when continuing as well as switching Dario Faggioli
@ 2017-03-01 15:34 ` Dario Faggioli
7 siblings, 0 replies; 13+ messages in thread
From: Dario Faggioli @ 2017-03-01 15:34 UTC (permalink / raw)
To: Jan Beulich; +Cc: George Dunlap, xen-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 860 bytes --]
On Wed, 2017-03-01 at 15:52 +0100, Dario Faggioli wrote:
> Dario Faggioli (7):
> xen: credit2: always mark a tickled pCPU as... tickled!
> xen: credit2: don't miss accounting while doing a credit reset.
>
And these two being bugfixes, I request them to be backported to Xen
4.7 (but not any further, as that would be difficult and pretty
pointless).
Since that isn't exactly trivial, especially for the first patch, I
attach to this email what I actually propose to backport.
I've successfully tested them on top of current tip of staging-4.7.
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.1.2: xen-credit2-always-mark.patch --]
[-- Type: text/x-patch, Size: 2224 bytes --]
commit eb0c1b77cc65ad40efd61234f8eb0dc73cd80632
Author: Dario Faggioli <dario.faggioli@citrix.com>
Date: Wed Mar 1 16:00:53 2017 +0100
xen: credit2: always mark a tickled pCPU as... tickled!
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.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 35dad15..f14dc9c 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -559,6 +559,13 @@ __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);
+ cpu_raise_softirq(cpu, SCHEDULE_SOFTIRQ);
+}
+
/* Check to see if the item on the runqueue is higher priority than what's
* currently running; if so, wake up the processor */
static /*inline*/ void
@@ -660,9 +667,8 @@ tickle:
sizeof(d),
(unsigned char *)&d);
}
- cpumask_set_cpu(ipid, &rqd->tickled);
SCHED_STAT_CRANK(tickle_idlers_some);
- cpu_raise_softirq(ipid, SCHEDULE_SOFTIRQ);
+ tickle_cpu(ipid, rqd);
no_tickle:
return;
@@ -1027,7 +1033,7 @@ 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) )
{
BUG_ON(svc->rqd != RQD(ops, vc->processor));
@@ -1308,8 +1314,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
{
[-- Attachment #1.1.3: xen-credit2-don-t-miss.patch --]
[-- Type: text/x-patch, Size: 3514 bytes --]
commit f36ce03fffab7526d9c94b46028a27a752e3f60e
Author: Dario Faggioli <dario.faggioli@citrix.com>
Date: Wed Mar 1 16:22:43 2017 +0100
xen: credit2: don't miss accounting while doing a credit reset.
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>
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 905fb12..c86f548 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -707,14 +707,23 @@ 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;
BUG_ON( is_idle_vcpu(svc->vcpu) );
BUG_ON( 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
[-- 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
^ permalink raw reply related [flat|nested] 13+ messages in thread