On Thu, Jul 23, 2015 at 7:22 PM, Rafael J. Wysocki <raf...@kernel.org> wrote: > Hi Viresh, > > On Thu, Jul 23, 2015 at 8:09 AM, Viresh Kumar <viresh.ku...@linaro.org> wrote: >> On 22-07-15, 18:42, Rafael J. Wysocki wrote: >>> > 3. 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 -- 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/
