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

[Rafael J. Wysocki]

Hi Viresh,

On Wed, Jul 29, 2015 at 4:21 PM, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> On 29-07-15, 15:57, Rafael J. Wysocki wrote:
>> In practice, this means a cpufreq driver registration done in parallel
>> with CPU hotplug.
>
> Not necessarily. Also consider the case where cpufreq driver is already working
> for a set of CPUs. And a new set of CPUs (that will share the policy) are
> getting physically added to the system.

That's what I mean by "hotplug" (as opposed to online/offline).

Adding a CPU involves (a) updating the present map and (b) registering
the CPU device and both are carried out together with CPU
online/offline locked.  At least that's the expectation and I'm not
aware of any architecture doing that differently.

So locking CPU online/offline around the cpufreq driver registration
would close that race and there's one more reason for doing that
(other than we do it for the cpufreq driver unregistration as I said
before): we need the CPU *online* map to be stable too while we're
registering the cpufreq driver.

> Anyway, even if there is no problem at all, I do agree with Russell that it will
> be better to do operations on behalf of the devices only when we are requested
> for those devices.

Problem is, we can't do that for all of them.  If the CPU is ofline
while being registered (the common case for hot-add) and it doesn't
have a policy already, we can't link it to an existing policy anyway,
so that operation has to be carried out later.  That is, we need to
create links for those CPUs after the policy has been created in any
case.

Moreover, the only case when we need to create links for online CPUs
is the registration of a cpufreq driver, because only then we can see
online CPUs that should be linked to a policy, but aren't.  It takes
less code to treat them in the same way as the offline CPUs at that
point (and create the links for them right after the policy has been
created) than to defer it to until sif->add() is called for each of
them, because we know that sif->add() is practically guaraneed to
succeeed for them (this is the code path in which we call
cpufreq_add_policy_cpu() and the governor "stop" only fails if it
hasn't been started for that policy).

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/