| Message ID | 147145426725.25877.3430896482552948117.stgit@Solace.fritz.box (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2016-08-17 at 19:17 +0200, Dario Faggioli wrote: > If there are idle pcpus inside the waking vcpu's > soft-affinity mask, we should really tickle one > of them (this is one of the purposes of the > __runq_tickle() function itself!), not just > any idle pcpu. > > The issue has been introduced in 02ea5031825d > ("credit1: properly deal with pCPUs not in any cpupool"), > where the usage of idle_mask is changed, without > updating the bottom of the function, where it > is also referenced. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> > --- > Cc: George Dunlap <george.dunlap@citrix.eu.com> > Oops! Sorry George, got your email address wrong for this one. Dario > Cc: Anshul Makkar <anshul.makkar@citrix.com> > Cc: David Vrabel <david.vrabel@citrix.com> > --- > David, a while ago you asked what could have been that was causing > awful > results for Credit1, for CPU bound workloads, in the overloaded > scenario of one > of my benchmarks. I think the bug fixed either here or in next patch > (but I'd > be rather sure it's this one) is where the problem was. :-) > --- > xen/common/sched_credit.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c > index 6eccf09..3d4f223 100644 > --- a/xen/common/sched_credit.c > +++ b/xen/common/sched_credit.c > @@ -454,11 +454,12 @@ static inline void __runq_tickle(struct > csched_vcpu *new) > if ( opt_tickle_one_idle ) > { > this_cpu(last_tickle_cpu) = > - cpumask_cycle(this_cpu(last_tickle_cpu), > &idle_mask); > + cpumask_cycle(this_cpu(last_tickle_cpu), > + cpumask_scratch_cpu(cpu)); > __cpumask_set_cpu(this_cpu(last_tickle_cpu), > &mask); > } > else > - cpumask_or(&mask, &mask, &idle_mask); > + cpumask_or(&mask, &mask, > cpumask_scratch_cpu(cpu)); > } > > /* Did we find anyone? */ >
On Thu, Aug 18, 2016 at 12:42 AM, Dario Faggioli <dario.faggioli@citrix.com> wrote: > On Wed, 2016-08-17 at 19:17 +0200, Dario Faggioli wrote: >> If there are idle pcpus inside the waking vcpu's >> soft-affinity mask, we should really tickle one >> of them (this is one of the purposes of the >> __runq_tickle() function itself!), not just >> any idle pcpu. >> >> The issue has been introduced in 02ea5031825d >> ("credit1: properly deal with pCPUs not in any cpupool"), >> where the usage of idle_mask is changed, without >> updating the bottom of the function, where it >> is also referenced. >> >> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Acked-by: George Dunlap <george.dunlap@citrix.com>
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index 6eccf09..3d4f223 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -454,11 +454,12 @@ static inline void __runq_tickle(struct csched_vcpu *new) if ( opt_tickle_one_idle ) { this_cpu(last_tickle_cpu) = - cpumask_cycle(this_cpu(last_tickle_cpu), &idle_mask); + cpumask_cycle(this_cpu(last_tickle_cpu), + cpumask_scratch_cpu(cpu)); __cpumask_set_cpu(this_cpu(last_tickle_cpu), &mask); } else - cpumask_or(&mask, &mask, &idle_mask); + cpumask_or(&mask, &mask, cpumask_scratch_cpu(cpu)); } /* Did we find anyone? */
If there are idle pcpus inside the waking vcpu's soft-affinity mask, we should really tickle one of them (this is one of the purposes of the __runq_tickle() function itself!), not just any idle pcpu. The issue has been introduced in 02ea5031825d ("credit1: properly deal with pCPUs not in any cpupool"), where the usage of idle_mask is changed, without updating the bottom of the function, where it is also referenced. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> --- Cc: George Dunlap <george.dunlap@citrix.eu.com> Cc: Anshul Makkar <anshul.makkar@citrix.com> Cc: David Vrabel <david.vrabel@citrix.com> --- David, a while ago you asked what could have been that was causing awful results for Credit1, for CPU bound workloads, in the overloaded scenario of one of my benchmarks. I think the bug fixed either here or in next patch (but I'd be rather sure it's this one) is where the problem was. :-) --- xen/common/sched_credit.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)