| Message ID | 146859415105.10217.6564561975071969210.stgit@Solace.fritz.box (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15/07/16 15:49, Dario Faggioli wrote: > In both Credit1 and Credit2, stop considering a pCPU idle, > if the reason why the idle vCPU is being selected, is to > do tasklet work. > > Not doing so means that the tickling and load balancing > logic, seeing the pCPU as idle, considers it a candidate > for picking up vCPUs. But the pCPU won't actually pick > up or schedule any vCPU, which would then remain in the > runqueue, which is bad, especially if there were other, > truly idle pCPUs, that could execute it. > > The only drawback is that we can't assume that a pCPU is > in always marked as idle when being removed from an > instance of the Credit2 scheduler (csched2_deinit_pdata). > In fact, if we are in stop-machine (i.e., during suspend > or shutdown), the pCPUs are running the stopmachine_tasklet > and hence are actually marked as busy. On the other hand, > when removing a pCPU from a Credit2 pool, it will indeed > be idle. The only thing we can do, therefore, is to > remove the BUG_ON() check. > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com> > --- > Cc: George Dunlap <george.dunlap@citrix.com> > Cc: Jan Beulich <JBeulich@suse.com> > Cc: Anshul Makkar <anshul.makkar@citrix.com> > Cc: David Vrabel <david.vrabel@citrix.com> > --- > Changes in v1: > * reorg. the Credit1 code for minimal churn, as requested during review; > * don't move out: label up in Credit1, as requested during review. > --- > As code changed, I did not apply Anshul's R-b tag. Anshul, re-issue it if you > want (if you feel like it, of course). > --- > xen/common/sched_credit.c | 2 +- > xen/common/sched_credit2.c | 14 ++++++++++---- > 2 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c > index ac22746..d547716 100644 > --- a/xen/common/sched_credit.c > +++ b/xen/common/sched_credit.c > @@ -1827,7 +1827,7 @@ csched_schedule( > * Update idlers mask if necessary. When we're idling, other CPUs > * will tickle us when they get extra work. > */ > - if ( snext->pri == CSCHED_PRI_IDLE ) > + if ( !tasklet_work_scheduled && snext->pri == CSCHED_PRI_IDLE ) > { > if ( !cpumask_test_cpu(cpu, prv->idlers) ) > cpumask_set_cpu(cpu, prv->idlers); > diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c > index 8b95a47..7e572bf 100644 > --- a/xen/common/sched_credit2.c > +++ b/xen/common/sched_credit2.c > @@ -1923,8 +1923,16 @@ csched2_schedule( > } > else > { > - /* Update the idle mask if necessary */ > - if ( !cpumask_test_cpu(cpu, &rqd->idle) ) > + /* > + * Update the idle mask if necessary. Note that, if we're scheduling > + * idle in order to carry on some tasklet work, we want to play busy! > + */ > + if ( tasklet_work_scheduled ) > + { > + if ( cpumask_test_cpu(cpu, &rqd->idle) ) > + cpumask_clear_cpu(cpu, &rqd->idle); > + } > + else if ( !cpumask_test_cpu(cpu, &rqd->idle) ) > cpumask_set_cpu(cpu, &rqd->idle); > /* Make sure avgload gets updated periodically even > * if there's no activity */ > @@ -2304,8 +2312,6 @@ csched2_deinit_pdata(const struct scheduler *ops, void *pcpu, int cpu) > /* No need to save IRQs here, they're already disabled */ > spin_lock(&rqd->lock); > > - BUG_ON(!cpumask_test_cpu(cpu, &rqd->idle)); > - > printk("Removing cpu %d from runqueue %d\n", cpu, rqi); > > cpumask_clear_cpu(cpu, &rqd->idle); >
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c index ac22746..d547716 100644 --- a/xen/common/sched_credit.c +++ b/xen/common/sched_credit.c @@ -1827,7 +1827,7 @@ csched_schedule( * Update idlers mask if necessary. When we're idling, other CPUs * will tickle us when they get extra work. */ - if ( snext->pri == CSCHED_PRI_IDLE ) + if ( !tasklet_work_scheduled && snext->pri == CSCHED_PRI_IDLE ) { if ( !cpumask_test_cpu(cpu, prv->idlers) ) cpumask_set_cpu(cpu, prv->idlers); diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c index 8b95a47..7e572bf 100644 --- a/xen/common/sched_credit2.c +++ b/xen/common/sched_credit2.c @@ -1923,8 +1923,16 @@ csched2_schedule( } else { - /* Update the idle mask if necessary */ - if ( !cpumask_test_cpu(cpu, &rqd->idle) ) + /* + * Update the idle mask if necessary. Note that, if we're scheduling + * idle in order to carry on some tasklet work, we want to play busy! + */ + if ( tasklet_work_scheduled ) + { + if ( cpumask_test_cpu(cpu, &rqd->idle) ) + cpumask_clear_cpu(cpu, &rqd->idle); + } + else if ( !cpumask_test_cpu(cpu, &rqd->idle) ) cpumask_set_cpu(cpu, &rqd->idle); /* Make sure avgload gets updated periodically even * if there's no activity */ @@ -2304,8 +2312,6 @@ csched2_deinit_pdata(const struct scheduler *ops, void *pcpu, int cpu) /* No need to save IRQs here, they're already disabled */ spin_lock(&rqd->lock); - BUG_ON(!cpumask_test_cpu(cpu, &rqd->idle)); - printk("Removing cpu %d from runqueue %d\n", cpu, rqi); cpumask_clear_cpu(cpu, &rqd->idle);
In both Credit1 and Credit2, stop considering a pCPU idle, if the reason why the idle vCPU is being selected, is to do tasklet work. Not doing so means that the tickling and load balancing logic, seeing the pCPU as idle, considers it a candidate for picking up vCPUs. But the pCPU won't actually pick up or schedule any vCPU, which would then remain in the runqueue, which is bad, especially if there were other, truly idle pCPUs, that could execute it. The only drawback is that we can't assume that a pCPU is in always marked as idle when being removed from an instance of the Credit2 scheduler (csched2_deinit_pdata). In fact, if we are in stop-machine (i.e., during suspend or shutdown), the pCPUs are running the stopmachine_tasklet and hence are actually marked as busy. On the other hand, when removing a pCPU from a Credit2 pool, it will indeed be idle. The only thing we can do, therefore, is to remove the BUG_ON() check. Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> --- Cc: George Dunlap <george.dunlap@citrix.com> Cc: Jan Beulich <JBeulich@suse.com> Cc: Anshul Makkar <anshul.makkar@citrix.com> Cc: David Vrabel <david.vrabel@citrix.com> --- Changes in v1: * reorg. the Credit1 code for minimal churn, as requested during review; * don't move out: label up in Credit1, as requested during review. --- As code changed, I did not apply Anshul's R-b tag. Anshul, re-issue it if you want (if you feel like it, of course). --- xen/common/sched_credit.c | 2 +- xen/common/sched_credit2.c | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-)