Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 10 Oct 2022 11:09:02 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     Lukasz Luba <lukasz.luba@arm.com>, peterz@infradead.org,
        Vincent Guittot <vincent.guittot@linaro.org>
Cc:     linux-kernel@vger.kernel.org, rafael@kernel.org,
        linux-pm@vger.kernel.org, Dietmar.Eggemann@arm.com
Subject: Re: [PATCH 2/2] cpufreq: Update CPU capacity reduction in
 store_scaling_max_freq()
Message-ID: <20221010053902.5rofnpzvyynumw3e@vireshk-i7>
References: <20220930094821.31665-1-lukasz.luba@arm.com>
 <20220930094821.31665-2-lukasz.luba@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220930094821.31665-2-lukasz.luba@arm.com>
Precedence: bulk

Would be good to always CC Scheduler maintainers for such a patch.

On 30-09-22, 10:48, Lukasz Luba wrote:
> When the new max frequency value is stored, the task scheduler must
> know about it. The scheduler uses the CPUs capacity information in the
> task placement. Use the existing mechanism which provides information
> about reduced CPU capacity to the scheduler due to thermal capping.
> 
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  drivers/cpufreq/cpufreq.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 1f8b93f42c76..205d9ea9c023 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -27,6 +27,7 @@
>  #include <linux/slab.h>
>  #include <linux/suspend.h>
>  #include <linux/syscore_ops.h>
> +#include <linux/thermal.h>
>  #include <linux/tick.h>
>  #include <linux/units.h>
>  #include <trace/events/power.h>
> @@ -718,6 +719,8 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf)
>  static ssize_t store_scaling_max_freq
>  (struct cpufreq_policy *policy, const char *buf, size_t count)
>  {
> +	unsigned int frequency;
> +	struct cpumask *cpus;
>  	unsigned long val;
>  	int ret;
>  
> @@ -726,7 +729,20 @@ static ssize_t store_scaling_max_freq
>  		return -EINVAL;
>  
>  	ret = freq_qos_update_request(policy->max_freq_req, val);
> -	return ret >= 0 ? count : ret;
> +	if (ret >= 0) {
> +		/*
> +		 * Make sure that the task scheduler sees these CPUs
> +		 * capacity reduction. Use the thermal pressure mechanism
> +		 * to propagate this information to the scheduler.
> +		 */
> +		cpus = policy->related_cpus;

No need of this, just use related_cpus directly.

> +		frequency = __resolve_freq(policy, val, CPUFREQ_RELATION_HE);
> +		arch_update_thermal_pressure(cpus, frequency);

I wonder if using the thermal-pressure API here is the right thing to
do. It is a change coming from User, which may or may not be
thermal-related.

> +
> +		ret = count;
> +	}
> +
> +	return ret;
>  }
>  
>  static ssize_t store_scaling_min_freq
> -- 
> 2.17.1

-- 
viresh