On 07/29/2013 05:19 PM, Rafael J. Wysocki wrote: > On Monday, July 29, 2013 03:11:45 PM Srivatsa S. Bhat wrote: >> On 07/29/2013 04:50 AM, Rafael J. Wysocki wrote: >>> On Monday, July 29, 2013 12:43:59 AM Rafael J. Wysocki wrote: >>>> On Monday, July 29, 2013 12:11:18 AM Rafael J. Wysocki wrote: >>>>> On Sunday, July 28, 2013 12:21:22 PM Toralf Förster wrote: >>>>>> On 07/28/2013 01:39 AM, Rafael J. Wysocki wrote: >>>>>>> On Saturday, July 27, 2013 07:40:34 PM Toralf Förster wrote: >>>>>>>> it gives at a ThinkPad T420: >>>>>>>> >>>>>>>> tfoerste@n22 ~/tmp $ lsmod | grep ^acpi_cpufreq >>>>>>>> acpi_cpufreq 12902 2147483647 >>>>>>> >>>>>>> That is -1, which indicates some module refcount woes. >>>>>> >>>>>> yes, ofc. >>>>>> >>>>>> The issue apears after 1 s2ram/resume cycle, before s2ram the refcount is 1. >>>>>> >>>>>>> I definitely can't see that with the mainline on my machines. >>>>>> >>>>>> It is in mainline too. >>>>> >>>>> Does the appended patch help? >>>> >>>> Actually, something as simple as this also should help: >>>> >>>> --- >>>> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> >>>> Subject: cpufreq: Fix cpufreq driver module refcount balance after suspend/resume >>>> >>>> Since cpufreq_cpu_put() called by __cpufreq_remove_dev() drops the >>>> driver module refcount, __cpufreq_remove_dev() causes that refcount >>>> to become negative after a suspend/resume cycle, for example. >>>> >>>> To prevent this from happening make __cpufreq_remove_dev() put >>>> the policy kobject only instead of calling cpufreq_cpu_put(). >>> >>> Having a deeper look at it, though, I see that in fact the whole >>> cpufreq_cpu_put() is needed if the CPU is not the last one for the given >>> policy and is not needed at all otherwise (as described in the changelog >>> of the patch below). >>> >>> Srivatsa, does this make sense to you? >>> >> >> Code-wise, your patch looks good to me. But one thing in the existing code >> struck me as a little strange. >> >> I'm assuming that the module_get() is used in the cpufreq core to ensure that >> until the cpufreq core is managing (atleast one) CPU(s), the cpufreq backend >> driver module (eg: acpi-cpufreq) can't be removed. > > Quite frankly, I'm not sure about that. If that were the case, > cpufreq_add_dev() would not call module_put() which it does. > > That may be a bug, I agree, but that's not for the present release cycle. For > now, we need to ensure that the reference counts are *balanced* . > Sure, in that case, I agree that your patch is the right fix at this point, since it resolves the immediate problem that we have with the refcounts. Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@xxxxxxxxxxxxxxxxxx> Regards, Srivatsa S. Bhat -- To unsubscribe from this list: send the line "unsubscribe cpufreq" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
