On Fri, Mar 4, 2016 at 11:18 PM, Steve Muckle [off-list ref] wrote:

On 03/03/2016 07:07 PM, Rafael J. Wysocki wrote:

quoted

+void cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
+                             unsigned int target_freq, unsigned int relation)
+{
+     unsigned int freq;
+
+     freq = cpufreq_driver->fast_switch(policy, target_freq, relation);
+     if (freq != CPUFREQ_ENTRY_INVALID) {
+             policy->cur = freq;
+             trace_cpu_frequency(freq, smp_processor_id());
+     }
+}

Even if there are platforms which may change the CPU frequency behind
cpufreq's back, breaking the transition notifiers, I'm worried about the
addition of an interface which itself breaks them. The platforms which
do change CPU frequency on their own have probably evolved to live with
or work around this behavior. As other platforms migrate to fast
frequency switching they might be surprised when things don't work as
advertised.

Well, intel_pstate doesn't do notifies at all, so anything depending
on them is already broken when it is used.  Let alone the hardware
P-states coordination mechanism (HWP) where the frequency is
controlled by the processor itself entirely.

That said I see your point.

I'm not sure what the easiest way to deal with this is. I see the
transition notifiers are the srcu type, which I understand to be
blocking. Going through the tree and reworking everyone's callbacks and
changing the type to atomic is obviously not realistic.

Right.

How about modifying cpufreq_register_notifier to return an error if the
driver has a fast_switch callback installed and an attempt to register a
transition notifier is made?

That sounds like a good idea.

There also is the CPUFREQ_ASYNC_NOTIFICATION driver flag that in
principle might be used as a workaround, but I'm not sure how much
work that would require ATM.

In the future, perhaps an additional atomic transition callback type can
be added, which platform/driver owners can switch to if they wish to use
fast transitions with their platform.

I guess you mean an atomic notification mechanism based on registering
callbacks?  While technically viable that's somewhat risky, because we
are in a fast path and allowing anyone to add stuff to it would be
asking for trouble IMO.

Thanks,
Rafael