Re: [PATCH v2 1/9] sched/uclamp: Fix relationship between uclamp and migration margin

[Dietmar Eggemann <dietmar.eggemann@arm.com>]

On 04/08/2022 16:36, Qais Yousef wrote:

[...]

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 52dc9d6f811e..00c2de800685 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4254,6 +4254,129 @@ static inline void util_est_update(struct cfs_rq *cfs_rq,
>  	trace_sched_util_est_se_tp(&p->se);
>  }
>  
> +static inline int util_fits_cpu(unsigned long util,
> +				unsigned long uclamp_min,
> +				unsigned long uclamp_max,
> +				int cpu)
> +{
> +	unsigned long capacity_orig, capacity_orig_thermal;
> +	unsigned long capacity = capacity_of(cpu);
> +	bool fits, uclamp_max_fits;
> +
> +	/*
> +	 * Check if the real util fits without any uclamp boost/cap applied.
> +	 */
> +	fits = fits_capacity(util, capacity);
> +
> +	if (!uclamp_is_used())
> +		return fits;
> +
> +	/*
> +	 * We must use capacity_orig_of() for comparing against uclamp_min and
> +	 * uclamp_max. We only care about capacity pressure (by using
> +	 * capacity_of()) for comparing against the real util.
> +	 *
> +	 * If a task is boosted to 1024 for example, we don't want a tiny
> +	 * pressure to skew the check whether it fits a CPU or not.
> +	 *
> +	 * Similarly if a task is capped to capacity_orig_of(little_cpu), it
> +	 * should fit a little cpu even if there's some pressure.
> +	 *
> +	 * Only exception is for thermal pressure since it has a direct impact
> +	 * on available OPP of the system.
> +	 *
> +	 * We honour it for uclamp_min only as a drop in performance level
> +	 * could result in not getting the requested minimum performance level.
> +	 *
> +	 * For uclamp_max, we can tolerate a drop in performance level as the
> +	 * goal is to cap the task. So it's okay if it's getting less.
> +	 *
> +	 * In case of capacity inversion, which is not handled yet, we should
> +	 * honour the inverted capacity for both uclamp_min and uclamp_max all
> +	 * the time.
> +	 */
> +	capacity_orig = capacity_orig_of(cpu);
> +	capacity_orig_thermal = capacity_orig - arch_scale_thermal_pressure(cpu);

Did you and Vincent agreed at the end to use `cap_orig - _instantaneous_
thermal pressure` (1) here?

Last email in v1 from Vincent on this one was
https://lkml.kernel.org/r/20220722151300.GA30193@vingu-book :

"Using capacity_orig_of(cpu) - thermal_load_avg(rq_of(cpu)) seems like
a simple solution to cover thermal mitigation".

And there is no Acked-By/Reviewed-By so far.

We use (1) in feec() to cater for the thermal throttling (thermal
restricting policy->max) schedutil takes into account immediately when
asking for frequency (performance). EAS and schedutil should see the
same thing.

Do you want to use the same in util_fits_cpu() since it's used in feec()?

[...]