Messages in this thread | | | Date | Mon, 02 Feb 2015 12:45:58 +0800 | From | ethan zhao <> | Subject | Re: [PATCH Resend] cpufreq: Set cpufreq_cpu_data to NULL before putting kobject |
| |
On 2015/2/2 12:26, Viresh Kumar wrote: > On 2 February 2015 at 09:46, ethan zhao <ethan.zhao@oracle.com> wrote: >> We am talking about the policy allocation and de-allocation. right ? >> I showed you the cpufreq_policy_free(policy) doesn't check kobject >> refcount as above. >> >> Hmmm, you are still sleeping in the kobject, wake up and don't mix >> water anymore. > It would be nice if we give each other the respect we deserve, And talk > about concrete points here. Welcome back to the right way. >>> if (!cpufreq_suspended) >>> cpufreq_policy_put_kobj(policy); >>> >>> static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy) >>> { >>> ... >>> >>> kobject_put(kobj); >>> >>> /* >>> * We need to make sure that the underlying kobj is >>> * actually not referenced anymore by anybody before we >>> * proceed with unloading. >>> */ >>> pr_debug("waiting for dropping of refcount\n"); >>> wait_for_completion(cmp); >>> } > I gave you exactly what you wanted to go through, but it seems you > haven't tried enough. > > Before freeing policy with cpufreq_policy_free(), we call > cpufreq_policy_put_kobj(). Now what does this function do? It waits > for the completion to fire (wait_for_completion()). This completion > will only fire when refcount of a kobject becomes zero. > > Initially when we create the kobject, it is initialized to one. And the > last kobject_put() you see above in cpufreq_policy_put_kobj() > makes it zero. All other cpufreq_cpu_get() and put() should happen > in pairs, otherwise this refcount will never be zero again. > > As soon as the refcount becomes zero, we are sure no one else is > using the policy structure anymore. And so we free it with > cpufreq_policy_free(). But there is no checking against refcount in or before
cpufreq_policy_free(), that is one issue I mentioned.
> > That routines doesn't have any tricks and simply frees the policy. > Because, before calling cpufreq_policy_put_kobj(), we have set > the per-cpu variable to NULL, nobody else will get the policy It is possible cpufreq_cpu_get() within the PPC thread was called just before __cpufreq_remove_dev_finish() is to be called in another thread, so you set the per_cpu(cpufreq_cpu_data, cpu) to NULL will not prevent the actions between cpufreq_cpu_get and cpufreq_cpu_put().
And then the freeing happens in __cpufreq_remove_dev_finish(). > structure by calling cpufreq_cpu_get(). And that's what my patch > tried to solve. > > Let me know if I wasn't explanatory enough..
Ethan > > -- > viresh
| |