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.