On Thu, Jul 23, 2015 at 7:22 PM, Rafael J. Wysocki rafael@kernel.org wrote:
Hi Viresh,
On Thu, Jul 23, 2015 at 8:09 AM, Viresh Kumar viresh.kumar@linaro.org wrote:
On 22-07-15, 18:42, Rafael J. Wysocki wrote:
- what happens when 'policy' is NULL at the point when the first (few) CPUs are added - how do the symlinks get created later if/when policy becomes non-NULL (can it?)
Yes, it can, and we have a design issue here that bothers me a bit.
I replied to Russell with a NO here as the first CPU should have created the policy. BUT...
Namley, we need a driver's ->init callback to populate policy->cpus for us, but this is not the only thing it is doing, so the concern is that it may not be able to deal with CPUs that aren't online.
... the first few CPUs could have been offline and so we might not have tried to add the policy at all.. Need to fix that for sure.
Wait here.
The current Linus' tree doesn't have that problem as far as I can say.
Say cpufreq_interface->add_dev() is called for an offline CPU (say CPU2). It points to cpufreq_add_dev(), so we see that the CPU is offline and call add_cpu_dev_symlink() for it. But the first argument we pass to that is per_cpu(cpufreq_cpu_data, cpu) and that is NULL, because the policy is not there yet. So we just return 0 (and the CPU has no policy and no link).
Now say cpufreq_interface->add_dev() is called for an online CPU (say CPU3). It goes and creates the policy for it and the driver's ->init() tells us that CPU2 is related to it. So cpufreq_add_dev_interface() creates the link for CPU2 and we're fine.
Now say CPU3 was offline too when cpufreq_interface->add_dev() was called for it. We don't create a policy or a link for it. Now say CPU2 becomes online. cpufreq_cpu_callback() calls cpufreq_add_dev() for it and we land in the previous case.
The *broken* case is when CPU2 is online to start with and it had created the link for CPU3, so when an offline CPU3 is now being added, we try to create the link for it again. That is the case we need to address in -rc without introducing new problems. The $subject patch adresses that issue, but it introduces the above problem. On the other hand, my patch at https://patchwork.kernel.org/patch/6839151/ should take care of this too (unless it is broken in a way I'm not seeing now).
It doesn't address the case when the CPU being removed is the policy owner.
Let me prepare a new version of it and we'll start over from there.
Thanks, Rafael