linux-kernel - Re: [PATCH v1] cpufreq: CPPC: Fix unused-function warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3a26ce90-5d2d-0164-3799-85a9dc1abee6@arm.com>
Date:   Mon, 30 May 2022 10:44:42 +0200
From:   Pierre Gondois <pierre.gondois@....com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     linux-kernel@...r.kernel.org, Ionela.Voinescu@....com,
        Dietmar.Eggemann@....com,
        Shaokun Zhang <zhangshaokun@...ilicon.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>, linux-pm@...r.kernel.org
Subject: Re: [PATCH v1] cpufreq: CPPC: Fix unused-function warning



On 5/30/22 10:20, Viresh Kumar wrote:
> On 30-05-22, 10:12, Pierre Gondois wrote:
>> Building the cppc_cpufreq driver with for arm64 with
>> CONFIG_ENERGY_MODEL=n triggers the following warnings:
>>   drivers/cpufreq/cppc_cpufreq.c:550:12: error: ‘cppc_get_cpu_cost’ defined but not used
>> [-Werror=unused-function]
>>     550 | static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
>>         |            ^~~~~~~~~~~~~~~~~
>>   drivers/cpufreq/cppc_cpufreq.c:481:12: error: ‘cppc_get_cpu_power’ defined but not used
>> [-Werror=unused-function]
>>     481 | static int cppc_get_cpu_power(struct device *cpu_dev,
>>         |            ^~~~~~~~~~~~~~~~~~
>>
>> Fixes: 740fcdc2c20e ("cpufreq: CPPC: Register EM based on efficiency class information")
>> Reported-by: Shaokun Zhang <zhangshaokun@...ilicon.com>
>> Signed-off-by: Pierre Gondois <pierre.gondois@....com>
>> ---
>>   drivers/cpufreq/cppc_cpufreq.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index d092c9bb4ba3..ecd0d3ee48c5 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -478,7 +478,7 @@ static inline unsigned long compute_cost(int cpu, int step)
>>   			step * CPPC_EM_COST_STEP;
>>   }
>>   
>> -static int cppc_get_cpu_power(struct device *cpu_dev,
>> +static __maybe_unused int cppc_get_cpu_power(struct device *cpu_dev,
>>   		unsigned long *power, unsigned long *KHz)
>>   {
>>   	unsigned long perf_step, perf_prev, perf, perf_check;
>> @@ -547,8 +547,8 @@ static int cppc_get_cpu_power(struct device *cpu_dev,
>>   	return 0;
>>   }
>>   
>> -static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
>> -		unsigned long *cost)
>> +static __maybe_unused int cppc_get_cpu_cost(struct device *cpu_dev,
>> +		unsigned long KHz, unsigned long *cost)
>>   {
>>   	unsigned long perf_step, perf_prev;
>>   	struct cppc_perf_caps *perf_caps;
> 
> Should we actually run cppc_cpufreq_register_em() for
> !CONFIG_ENERGY_MODEL ? Why?
> 

Hello Viresh,
It seems that when CONFIG_ENERGY_MODEL=n, the compiler is already
considering cppc_cpufreq_register_em() as an empty function.

Indeed, CONFIG_ENERGY_MODEL=n makes em_dev_register_perf_domain()
an empty function, so cppc_cpufreq_register_em() is only made of
variable definitions. This compiler optimization also explains
why cppc_get_cpu_power() and cppc_get_cpu_cost() trigger the
-Wunused-function warning.

Putting cppc_cpufreq_register_em() inside an
#ifdef CONFIG_ENERGY_MODEL
guard seems also valid to me. To avoid too many empty definitions
of cppc_cpufreq_register_em(), I guess it should be inside an
#if defined(CONFIG_ARM64) && defined(CONFIG_ENERGY_MODEL)
guard instead.
Please let me know what you prefer.

Regards,
Pierre