Thread (19 messages) 19 messages, 4 authors, 2015-07-30

Re: [PATCH v2] cpufreq: Avoid attempts to create duplicate symbolic links

From: Russell King - ARM Linux <hidden>
Date: 2015-07-29 09:11:54
Also in: lkml

On Wed, Jul 29, 2015 at 03:38:03AM +0200, Rafael J. Wysocki wrote:
On Monday, July 27, 2015 08:09:35 PM Viresh Kumar wrote:
quoted
On 27-07-15, 15:45, Rafael J. Wysocki wrote:
quoted
Say the subsys add callback runs for a CPU and it doesn't have a policy.
If it is offline, we ignore it and the add callback won't be executed
for it again.

In turn, if it is online, we create a policy for it and we should (right
away) link the policy to all of the CPUs that were offline when the subsys add
callback was called for them.  That's what we do today.

Is there anything missing in that?
So the code is working properly after your patch, but I was talking
on the lines of what Russell suggested.

We should play with the links only when we receive add-dev/remove-dev
from subsys callbacks. The exception to that will be the offline CPUs
for which add-dev is called before their policy existed.
The rule is supposed to be "all of the present CPUs which do not own
a policy should point to one, unless it doesn't exist".  The right
approach is then to create links from them to a policy object as soon
as we create one for them.  Waiting for something else to happen is just
pointless and this approach covers both the offline and online CPUs, so
I don't think that changing it would improve things really.
I'm not sure we disagree with that.  It's more about when the symlinks
are created, and when you define that a CPU exists.

If you're attaching to subsystem in sysfs, then the point that the
subsystem interface gets to know about a sysfs node existing is when
it's add_dev method is called - it should not assume that a node exists
prior to that point, otherwise things are racy.

Consider a policy initialisation in parallel with an update of the
CPU present map and adding a CPU to sysfs.  The CPU present map will be
updated first, and then it will be added to sysfs.  If you're initialising
a cpufreq policy in the middle of that and creating symlinks for all
present CPUs, there's a window where the CPU present map indicates that
a CPU is present, but there is no sysfs directory for you to create a
symlink in.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help