> 在 2014年7月11日，3:26，"Saravana Kannan" <skannan@xxxxxxxxxxxxxx> 写道：
>
>> On 07/10/2014 08:12 AM, Bu, Yitian wrote:
>>
>>
>>> 在 2014年7月10日，20:41，"Viresh Kumar" <viresh.kumar@xxxxxxxxxx> 写道：
>>>
>>> This is only relevant to implementations with multiple clusters, where clusters
>>> have separate clock lines but all CPUs within a cluster share it.
>>>
>>> Consider a dual cluster platform with 2 cores per cluster. During suspend we
>>> start offlining CPUs from 1 to 3. When CPU2 is remove, policy->kobj would be
>>> moved to CPU3 and when CPU3 goes down we wouldn't free policy or its kobj.
>>>
>>> Now on resume, we will get CPU2 before CPU3 and will call __cpufreq_add_dev().
>>> We will recover the old policy and update policy->cpu from 3 to 2 from
>>> update_policy_cpu().
>>>
>>> But the kobj is still tied to CPU3 and wasn't moved to CPU2. We wouldn't create
>>> a link for CPU2, but would try that while bringing CPU3 online. Which will
>>> report errors as CPU3 already has kobj assigned to it.
>>>
>>> This bug got introduced with commit 42f921a, which overlooked this scenario.
>>>
>>> To fix this, lets move kobj to the new policy->cpu while bringing first CPU of a
>>> cluster back. We already have update_policy_cpu() routine which can be updated
>>> with this change. That would get rid of the cpufreq_nominate_new_policy_cpu() as
>>> update_policy_cpu() is also called on CPU removal.
>>>
>>> To achieve that we add another parameter to update_policy_cpu() as cpu_dev is
>>> present with both the callers.
>>>
>>> Fixes: ("42f921a cpufreq: remove sysfs files for CPUs which failed to come back after resume")
>>> Cc: Stable <stable@xxxxxxxxxxxxxxx> # 3.13+
>>> Reported-by: Bu Yitian <ybu@xxxxxxxxxxxxxxxx>
>>> Reported-by: Saravana Kannan <skannan@xxxxxxxxxxxxxx>
>>> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
>>> ---
>>> V1->V2: Move kobject_move() call to update_policy_cpu(), which makes
>>> cpufreq_nominate_new_policy_cpu() almost empty. So we remove it completely.
>>>
>>> @Yitian: Sorry, but you need to test this again as there were enough
>>> modifications in V2.
>>
>> Sorry, I don't have latest kernel to test this patch...
>> What I am using is 3.10, this patch seems too big to manually apply to 3.10.
>
> Even though our kernel is 3.10, I believe we have pulled in all cpufreq
> up to 3.14. So, if you have time, you could pull in rest of the cpufreq
> changes and test it out. For our tree, maybe the v1 patch would be
> sufficient?
>
> -Saravana
V1 patch is sufficient, which is the same as my original patch.
I have already verified the V1 patch and it works.
>>
>>
>>> drivers/cpufreq/cpufreq.c | 73 +++++++++++++++++++++++------------------------
>>> 1 file changed, 36 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>>> index 62259d2..c81d9ec6 100644
>>> --- a/drivers/cpufreq/cpufreq.c
>>> +++ b/drivers/cpufreq/cpufreq.c
>>> @@ -1076,10 +1076,20 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
>>> kfree(policy);
>>> }
>>>
>>> -static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
>>> +static int update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu,
>>> + struct device *cpu_dev)
>>> {
>>> + int ret;
>>> +
>>> if (WARN_ON(cpu == policy->cpu))
>>> - return;
>>> + return 0;
>>> +
>>> + /* Move kobject to the new policy->cpu */
>>> + ret = kobject_move(&policy->kobj, &cpu_dev->kobj);
>>> + if (ret) {
>>> + pr_err("%s: Failed to move kobj: %d\n", __func__, ret);
>>> + return ret;
>>> + }
>>>
>>> down_write(&policy->rwsem);
>>>
>>> @@ -1090,6 +1100,8 @@ static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
>>>
>>> blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>>> CPUFREQ_UPDATE_POLICY_CPU, policy);
>>> +
>>> + return 0;
>>> }
>>>
>>> static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
>>> @@ -1154,7 +1166,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
>>> * by invoking update_policy_cpu().
>>> */
>>> if (recover_policy && cpu != policy->cpu)
>>> - update_policy_cpu(policy, cpu);
>>> + WARN_ON(update_policy_cpu(policy, cpu, dev));
>>> else
>>> policy->cpu = cpu;
>>>
>>> @@ -1307,38 +1319,11 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif)
>>> return __cpufreq_add_dev(dev, sif);
>>> }
>>>
>>> -static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy,
>>> - unsigned int old_cpu)
>>> -{
>>> - struct device *cpu_dev;
>>> - int ret;
>>> -
>>> - /* first sibling now owns the new sysfs dir */
>>> - cpu_dev = get_cpu_device(cpumask_any_but(policy->cpus, old_cpu));
>>> -
>>> - sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
>>> - ret = kobject_move(&policy->kobj, &cpu_dev->kobj);
>>> - if (ret) {
>>> - pr_err("%s: Failed to move kobj: %d\n", __func__, ret);
>>> -
>>> - down_write(&policy->rwsem);
>>> - cpumask_set_cpu(old_cpu, policy->cpus);
>>> - up_write(&policy->rwsem);
>>> -
>>> - ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj,
>>> - "cpufreq");
>>> -
>>> - return -EINVAL;
>>> - }
>>> -
>>> - return cpu_dev->id;
>>> -}
>>> -
>>> static int __cpufreq_remove_dev_prepare(struct device *dev,
>>> struct subsys_interface *sif)
>>> {
>>> unsigned int cpu = dev->id, cpus;
>>> - int new_cpu, ret;
>>> + int ret;
>>> unsigned long flags;
>>> struct cpufreq_policy *policy;
>>>
>>> @@ -1378,14 +1363,28 @@ static int __cpufreq_remove_dev_prepare(struct device *dev,
>>> if (cpu != policy->cpu) {
>>> sysfs_remove_link(&dev->kobj, "cpufreq");
>>> } else if (cpus > 1) {
>>> - new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu);
>>> - if (new_cpu >= 0) {
>>> - update_policy_cpu(policy, new_cpu);
>>> + /* Nominate new CPU */
>>> + int new_cpu = cpumask_any_but(policy->cpus, cpu);
>>> + struct device *cpu_dev = get_cpu_device(new_cpu);
>>>
>>> - if (!cpufreq_suspended)
>>> - pr_debug("%s: policy Kobject moved to cpu: %d from: %d\n",
>>> - __func__, new_cpu, cpu);
>>> + sysfs_remove_link(&cpu_dev->kobj, "cpufreq");
>>> + ret = update_policy_cpu(policy, new_cpu, cpu_dev);
>>> + if (ret) {
>>> + /*
>>> + * To supress compilation warning about return value of
>>> + * sysfs_create_link().
>>> + */
>>> + int temp;
>>> +
>>> + /* Create link again if we failed. */
>>> + temp = sysfs_create_link(&cpu_dev->kobj, &policy->kobj,
>>> + "cpufreq");
>>> + return ret;
>>> }
>>> +
>>> + if (!cpufreq_suspended)
>>> + pr_debug("%s: policy Kobject moved to cpu: %d from: %d\n",
>>> + __func__, new_cpu, cpu);
>>> } else if (cpufreq_driver->stop_cpu && cpufreq_driver->setpolicy) {
>>> cpufreq_driver->stop_cpu(policy);
>>> }
>>> --
>>> 2.0.0.rc2
>
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
?韬{.n?????%??檩??w?{.n????{???塄}?财??j:+v?? ? ???2??璀??摺?囤??z夸z罐?+?????w棹f