hi, Viresh
thanks for your quick reply :)
On 2015年07月07日 16:53, Viresh Kumar wrote:
> On 07-07-15, 15:52, Pan Xinhui wrote:
>> I have latest codes.
>> codes in cpufreq.c are below.
>> 1436 down_write(&policy->rwsem);
>> 1437 cpumask_clear_cpu(cpu, policy->cpus);
>> 1438
>> 1439 if (policy_is_inactive(policy)) {
>> 1440 if (has_target())
>> 1441 strncpy(policy->last_governor, policy->governor->name,
>> 1442 CPUFREQ_NAME_LEN);
>> 1443 } else if (cpu == policy->cpu) {
>> 1444 /* Nominate new CPU */
>> 1445 policy->cpu = cpumask_any(policy->cpus);
>> 1446 }
>> 1447 up_write(&policy->rwsem);
>
> Sigh. Too bad. So what has changed is that the sysfs directory is
> allocated to policy->cpu during init and never changed. But
> policy->cpu can surely change.
>
> Sorry for that.
>
That's OKay. You are very busy reviewing codes written by still fresh guys like
me.
>> back to my previous patch, you suggest me to use policy->driver_data to
>> *store* data and don't need use per_cpu anymore.
>> codes in acpi-cpufreq.c are below.
>> 365 static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
>> 366 {
>> 367 struct acpi_cpufreq_data *data = per_cpu(acfreq_data, cpu);
>> 368 unsigned int freq;
>> 369 unsigned int cached_freq;
>>
>> we get *data* through per_cpu for now, as the parameter is cpu only.
>> If we store *data* in policy->driver_data, we need call
>> struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu) to get policy.
>> We do a full codes review, and find there should be deadlock if we doing so.
>
> Why?
>
sorry, after double check. it's not caused by cpufreq_cpu_get.
I am working on several branches, these codes are little different, it's OKay
here.
Sorry for mistakes.
>> But as cpufreq code offers
>> 238 /* Only for cpufreq core internal use */
>> 239 struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu)
>>
>> I have a small question,if we can use *cpufreq_cpu_get_raw* in ->get
>> callback, which is already lock hold,
>> But the comment(line 238) is... hmm.
>
> That is more internal to the core. Better don't use it.
>
yes, *cpufreq_cpu_get* is OKay. thanks.
>> thanks for your kind reply. any advices or comments are welcome.
>
> Anyway, your patch is far from complete. You have just fixed a single
> place where per-cpu data is accessed with policy->cpu. What about rest
> of the code? Like target() :)
>
I have generated one patch which replacing all per_cpu with *driver_data*, it
works well in our Intel's branch for at least 2 days.
Let me do more codes review and tests before sending to LKML. :)
thanks for your advices :) it's really good.
thanks
xinhui
> --
> viresh
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
