Re: [PATCH V3 1/2] topology: Allow multiple entities to provide sched_freq_tick() callback

On Wednesday 17 Feb 2021 at 17:10:27 (+0530), Viresh Kumar wrote:

On 17-02-21, 11:30, Ionela Voinescu wrote:

quoted

The problem is not topology_scale_freq_invariant() but whether a scale
factor is set for some CPUs.

Scenario (test system above):
 - "AMUs" are only supported for [1-2],
 - cpufreq_supports_freq_invariance() -> false

What should happen:
 - topology_scale_freq_invariant() -> false (passed)
 - all CPUs should have their freq_scale unmodified (1024) - (failed)
   because only 2 out of 6 CPUs have a method of setting a scale factor

What does happen:
 - arch_set_freq_tick() -> topology_set_freq_tick() will set a scale
   factor for [1-2] based on AMUs. This should not happen. We will end
   up with invariant signals for bigs and signals that are not freq
   invariant for littles.

Another case. cpufreq is included as a module and AMU is implemented
partially.

- first time cpufreq driver is inserted, we set up everything and
  freq_scale gets updated on ticks.

- remove cpufreq driver, we are back in same situation.

Yes, but the littles (lacking AMUs) would have had a scale factor set
through arch_set_freq_scale() which will correspond to the last
frequency change through the cpufreq driver. When removing the driver,
it's unlikely that the frequency of littles will change (no driver).
 - topology_scale_freq_invariant() will still return true.
 - littles would still  have a scale factor set which is likely accurate
 - bigs will continue updating the scale factor through AMUs.

See a very useful comment someone added recently :) :

"""
+	/*
+	 * We don't need to handle CPUFREQ_REMOVE_POLICY event as the AMU
+	 * counters don't have any dependency on cpufreq driver once we have
+	 * initialized AMU support and enabled invariance. The AMU counters will
+	 * keep on working just fine in the absence of the cpufreq driver, and
+	 * for the CPUs for which there are no counters available, the last set
+	 * value of freq_scale will remain valid as that is the frequency those
+	 * CPUs are running at.
+	 */
"""

We can't control it that way.. Or we add another call layer in middle
before the tick-handler gets called for AMU, which will check if we
are fully invariant or not ?

I would avoid additional work done on the tick, especially for a scenario which
is unlikely. If you think this case is worth supporting, it might be best to do
it at CPUFREQ_REMOVE_POLICY event.

Thanks,
Ionela.

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel