| Message ID | 20160211113901.20959.87801.stgit@Solace.station (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 11.02.16 at 12:39, <dario.faggioli@citrix.com> wrote: > --- a/xen/common/sched_credit.c > +++ b/xen/common/sched_credit.c > @@ -1022,11 +1022,14 @@ csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc, unsigned wf) > * more CPU resource intensive VCPUs without impacting overall > * system fairness. > * > - * The one exception is for VCPUs of capped domains unpausing > - * after earning credits they had overspent. We don't boost > - * those. > + * There are a couple of exceptions, when we don't want to boost: > + * - VCPUs that are waking up after a migration, rather than > + * after having block; > + * - VCPUs of capped domains unpausing after earning credits > + * they had overspent. > */ > - if ( svc->pri == CSCHED_PRI_TS_UNDER && > + if ( !(wf & WF_migrated) && > + svc->pri == CSCHED_PRI_TS_UNDER && > !test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) ) > { Considering the other svc->flags check done here, wouldn't it be possible to achieve the same effect without patch 2, by having csched_cpu_pick() set a newly defined flag, and check for it here? Jan
On Thu, 2016-02-11 at 06:30 -0700, Jan Beulich wrote: > > > > On 11.02.16 at 12:39, <dario.faggioli@citrix.com> wrote: > > --- a/xen/common/sched_credit.c > > +++ b/xen/common/sched_credit.c > > @@ -1022,11 +1022,14 @@ csched_vcpu_wake(const struct scheduler > > *ops, struct vcpu *vc, unsigned wf) > > * more CPU resource intensive VCPUs without impacting > > overall > > * system fairness. > > * > > - * The one exception is for VCPUs of capped domains unpausing > > - * after earning credits they had overspent. We don't boost > > - * those. > > + * There are a couple of exceptions, when we don't want to > > boost: > > + * - VCPUs that are waking up after a migration, rather than > > + * after having block; > > + * - VCPUs of capped domains unpausing after earning credits > > + * they had overspent. > > */ > > - if ( svc->pri == CSCHED_PRI_TS_UNDER && > > + if ( !(wf & WF_migrated) && > > + svc->pri == CSCHED_PRI_TS_UNDER && > > !test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) ) > > { > > Considering the other svc->flags check done here, wouldn't it be > possible to achieve the same effect without patch 2, by having > csched_cpu_pick() set a newly defined flag, and check for it here? > It can indeed. I've coded it up, and I like the way it came out better. I'm rerunning the benchmarks right now (just in case! :-)). I'll send v2 out as soon as they finish. I did like the idea of "wakeup flags", and I think they may actually turn out useful, but they're not necessary for this specific use case, as it appears. Well, next time. ;-) Thanks and Regards, Dario
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index f728ddd..2a23a0b 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -1022,11 +1022,14 @@ csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc, unsigned wf) * more CPU resource intensive VCPUs without impacting overall * system fairness. * - * The one exception is for VCPUs of capped domains unpausing - * after earning credits they had overspent. We don't boost - * those. + * There are a couple of exceptions, when we don't want to boost: + * - VCPUs that are waking up after a migration, rather than + * after having block; + * - VCPUs of capped domains unpausing after earning credits + * they had overspent. */ - if ( svc->pri == CSCHED_PRI_TS_UNDER && + if ( !(wf & WF_migrated) && + svc->pri == CSCHED_PRI_TS_UNDER && !test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) ) { TRACE_2D(TRC_CSCHED_BOOST_START, vc->domain->domain_id, vc->vcpu_id); diff --git a/xen/common/schedule.c b/xen/common/schedule.c index ea74c96..c9a4f52 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -581,8 +581,8 @@ static void vcpu_migrate(struct vcpu *v) if ( old_cpu != new_cpu ) sched_move_irqs(v); - /* Wake on new CPU. */ - vcpu_wake(v); + /* Wake on new CPU (and let the scheduler know it's a migration). */ + _vcpu_wake(v, WF_migrated); } /* diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 9fdcfff..5f426ad 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -764,6 +764,9 @@ static inline struct domain *next_domain_in_cpupool( /* 'Default' wakeup. */ #define _WF_wakeup 0 #define WF_wakeup (1U<<_WF_wakeup) +/* Post-migration wakeup. */ +#define _WF_migrated 1 +#define WF_migrated (1U<<_WF_migrated) static inline int vcpu_runnable(struct vcpu *v) {
Moving a vCPU to a different pCPU means blocking it and waking it up (on the new pCPU). Credit1 grants BOOST priority to vCPUs that wakes up, with the aim of improving I/O latency. The end result is that vCPUs get boosted when migrating, which shouldn't happen. For instance, this causes scheduling anomalies and, potentially, performance problems, as reported here: http://lists.xen.org/archives/html/xen-devel/2015-10/msg02851.html This patch fixes things by introducing a new wakeup flag, and using it to tag the wakeups that happens because of migrations (and avoid boosting, in these cases). Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> --- Cc: George Dunlap <george.dunlap@eu.citrix.com> --- xen/common/sched_credit.c | 11 +++++++---- xen/common/schedule.c | 4 ++-- xen/include/xen/sched.h | 3 +++ 3 files changed, 12 insertions(+), 6 deletions(-)