lkml.org 
[lkml]   [2014]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 1/2] cpufreq: Don't destroy/realloc policy/sysfs on hotplug/suspend
On 07/16/2014 06:13 AM, Viresh Kumar wrote:
> On 16 July 2014 16:46, Srivatsa S. Bhat <srivatsa@mit.edu> wrote:
>> Short answer: If the sysfs directory has already been created by cpufreq,
>> then yes, it will remain as it is. However, if the online operation failed
>> before that, then cpufreq won't know about that CPU at all, and no file will
>> be created.
>>
>> Long answer:
>> The existing cpufreq code does all its work (including creating the sysfs
>> directories etc) at the CPU_ONLINE stage. This stage is not expected to fail
>> (in fact even the core CPU hotplug code in kernel/cpu.c doesn't care for
>> error returns at this point). So if a CPU fails to come up in earlier stages
>> itself (such as CPU_UP_PREPARE), then cpufreq won't even hear about that CPU,
>> and hence no sysfs files will be created/linked. However, if the CPU bringup
>> operation fails during the CPU_ONLINE stage after the cpufreq's notifier has
>> been invoked, then we do nothing about it and the cpufreq sysfs files will
>> remain.
>
> In short, the problem I mentioned before this para is genuine. And setting
> policy->cpu to the first cpu of a mask is indeed a bad idea.

No it's not. All the cpu*/ directories for all possible CPUs will be
there whether a CPU is online/offline. Which is why I also weed out
impossible CPUs, but you said the driver shouldn't be passing impossible
CPUs anyway. I'm just picking one directory to put the real cpufreq
directory under. So, the code as-is is definitely not broken.

Sure, I can pick the first cpu that comes online to decide where to put
the real sysfs cpufreq directory, but then I have to keep track of that
in a separate field when it's time to remove it when the cpufreq driver
is unregistered. It's yet another pointless thing to keep track. And no,
we shouldn't be moving sysfs directory to stay with only an online
directory. That's the thing this patch is trying to simplify.

So, I think using the first cpu in related CPUs will always work. If
there's any disagreement, I think it's purely a personal preference over
adding another field vs calling cpumask_first()

-Saravana

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


\
 
 \ /
  Last update: 2014-07-16 22:21    [W:0.310 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site