Re: [PATCH 10/12] cpufreq: governor: Don't WARN on invalid states

[Preeti U Murthy <preeti@linux.vnet.ibm.com>]

On 06/11/2015 04:21 PM, Viresh Kumar wrote:
> The last commit returned errors on invalid state requests for a
> governor. But we are already issuing a WARN for an invalid state in
> cpufreq_governor_dbs().
> 
> Lets stop warning on that until the time cpufreq core is fixed to
> serialize state changes of the governor.

The way I see it is that if this patch series manages to capture invalid
states right, we don't need to necessarily serialize state changes in
the cpufreq core to get rid of the race conditions. So getting a START
after an EXIT will not be fatal(WARN_ON() will trigger then), if it is
identified as an invalid operation at that point and prevented.

So yes, we must get rid of the WARN_ON() not because we want to
reintroduce it later when all races are fixed but because the condition
that it is warning on can happen even if we get this fixed right making
it essentially a false positive.

And its ok to get rid of the WARN_ON() now rather than waiting till all
races are fixed, because anyway we are bound to crash the moment we hit
the warning today and we will know anyway that things went out of hand :P
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq_governor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> index ee2e19a1218a..c26f535d3d91 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -542,7 +542,7 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
>  	else
>  		dbs_data = cdata->gdbs_data;
> 
> -	if (WARN_ON(!dbs_data && (event != CPUFREQ_GOV_POLICY_INIT))) {
> +	if (!dbs_data && (event != CPUFREQ_GOV_POLICY_INIT)) {
>  		ret = -EINVAL;
>  		goto unlock;
>  	}
> 

Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>