Re: [PATCH] sched/core: forced idle accounting

[Peter Zijlstra <peterz@infradead.org>]

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;
> +	}
> +}