Messages in this thread | | | Date | Sat, 9 Oct 2021 17:54:35 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH] sched/core: forced idle accounting |
| |
On Thu, Oct 07, 2021 at 05:08:25PM -0700, Josh Don wrote: > Adds accounting for "forced idle" time, which is time where a cookie'd > task forces its SMT sibling to idle, despite the presence of runnable > tasks. > > Forced idle time is one means to measure the cost of enabling core > scheduling (ie. the capacity lost due to the need to force idle).
It seems an excessive amount of code for what it says to do.
> + smt_count = cpumask_weight(smt_mask);
That's a fairly expensive operation to find a number that's going the be to same over and over and over...
> + if (smt_count > 2) { > + unsigned int nr_forced_idle = 0, nr_running = 0; > + > + for_each_cpu(i, smt_mask) { > + rq_i = cpu_rq(i); > + p = rq_i->core_pick ?: rq_i->curr; > + > + if (p != rq_i->idle) > + nr_running++; > + else if (rq_i->nr_running) > + nr_forced_idle++; > + } > + > + if (WARN_ON_ONCE(!nr_running)) { > + /* can't be forced idle without a running task */ > + } else { > + delta *= nr_forced_idle; > + delta /= nr_running; > + }
Now the comment sayeth:
> + /* > + * For larger SMT configurations, we need to scale the charged > + * forced idle amount since there can be more than one forced idle > + * sibling and more than one running cookied task. > + */
But why?
> + } > + > + for_each_cpu(i, smt_mask) { > + rq_i = cpu_rq(i); > + p = rq_i->core_pick ?: rq_i->curr; > + > + if (!p->core_cookie) > + continue; > + > + p->core_forceidle_sum += delta; > + > + /* Optimize for common case. */ > + if (smt_count == 2) > + break; > + } > +}
| |