Re: [PATCH 7/8] cpufreq: Preserve policy structure across suspend/resume

[Srivatsa S. Bhat]

On 07/15/2013 03:25 PM, Viresh Kumar wrote:
> Hi Srivatsa,
> 
> I may be wrong but it looks something is wrong in this patch.
> 
> On 12 July 2013 03:47, Srivatsa S. Bhat
> <srivatsa.bhat@xxxxxxxxxxxxxxxxxx> wrote:
>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> 
>> @@ -1239,29 +1263,40 @@ static int __cpufreq_remove_dev(struct device *dev,
>>         if ((cpus == 1) && (cpufreq_driver->target))
>>                 __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT);
>>
>> -       pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
>> -       cpufreq_cpu_put(data);
>> +       if (!frozen) {
>> +               pr_debug("%s: removing link, cpu: %d\n", __func__, cpu);
>> +               cpufreq_cpu_put(data);
> 
> So, we don't decrement usage count here. But we are still increasing
> counts on cpufreq_add_dev after resume, isn't it?
> 
> So, we wouldn't be able to free policy struct once all the cpus of a
> policy are removed after suspend/resume has happened once.
> 

Actually even I was wondering about this while writing the patch and
I even tested shutdown after multiple suspend/resume cycles, to verify that
the refcount is messed up. But surprisingly, things worked just fine.

Logically there should've been a refcount mismatch and things should have
failed, but everything worked fine during my tests. Apart from suspend/resume
and shutdown tests, I even tried mixing a few regular CPU hotplug operations
(echo 0/1 to sysfs online files), but nothing stood out.

Sorry, I forgot to document this in the patch. Either the patch is wrong
or something else is silently fixing this up. Not sure what is the exact
situation.

Regards,
Srivatsa S. Bhat

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/