On 14/06/17 14:08, Vincent Guittot wrote: > On 14 June 2017 at 09:55, Dietmar Eggemann <dietmar.eggem...@arm.com> wrote: >> >> On 06/12/2017 04:27 PM, Vincent Guittot wrote: >>> On 8 June 2017 at 09:55, Dietmar Eggemann <dietmar.eggem...@arm.com> wrote:
[...] >> >> Yes, we should free cpus_to_visit if the policy notifier registration >> fails. But IMHO also, once the parsing of the capacity-dmips-mhz property >> is done. free cpus_to_visit is only used in the notifier call >> init_cpu_capacity_callback() after being allocated and initialized in >> register_cpufreq_notifier(). >> >> We could add something like this as the first patch of this set. Only >> mildly tested on Juno. Juri, what do you think? >> >> Author: Dietmar Eggemann <dietmar.eggem...@arm.com> >> Date: Tue Jun 13 23:21:59 2017 +0100 >> >> drivers base/arch_topology: free cpumask cpus_to_visit >> >> Free cpumask cpus_to_visit in case registering >> init_cpu_capacity_notifier has failed or the parsing of the cpu >> capacity-dmips-mhz property is done. The cpumask cpus_to_visit is >> only used inside the notifier call init_cpu_capacity_callback. >> >> Reported-by: Vincent Guittot <vincent.guit...@linaro.org> >> Signed-off-by: Dietmar Eggemann <dietmar.eggem...@arm.com> > > your proposal for freeing cpus_to_visit looks good for me > > Acked-by: Vincent Guittot <vincent.guit...@linaro.org> Thanks. [...] >> IMHO, that's not necessary. >> >> The transition notifier works completely independent from the policy >> notifier. In case the latter gets registered correctly and the registration >> of the former fails, the notifier call of the policy notifier still parses >> the capacity-dmips-mhz property information and sets per_cpu(max_freq, cpu). >> >> The notifier call set_freq_scale_callback() of the transition notifier will >> not be called so that frequency invariance always returns >> SCHED_CAPACITY_SCALE. >> >> After the policy notifier has finished its work, it schedules >> parsing_done_work() in which it gets unregistered. > > Ok so IIUC, the transition notifier is somehow optional and we still > have the cpu invariance. > In this case, you should not return the error code of > cpufreq_register_notifier(&set_freq_scale_notifier, > CPUFREQ_TRANSITION_NOTIFIER) as the error code of the > register_cpufreq_notifier function. > you should better print a warning like " failed to init frequency > invariance" and return 0 for register_cpufreq_notifier() Makes sense. Will change this. [...]