Messages in this thread | | | Subject | Re: [PATCH v8 4/8] PM / EM: add support for other devices than CPUs in Energy Model | From | Lukasz Luba <> | Date | Tue, 2 Jun 2020 12:31:25 +0100 |
| |
Hi Daniel,
On 6/1/20 10:44 PM, Daniel Lezcano wrote: > On 27/05/2020 11:58, Lukasz Luba wrote: >> Add support for other devices than CPUs. The registration function >> does not require a valid cpumask pointer and is ready to handle new >> devices. Some of the internal structures has been reorganized in order to >> keep consistent view (like removing per_cpu pd pointers). >> >> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com> >> --- > > [ ... ] > >> } >> EXPORT_SYMBOL_GPL(em_register_perf_domain); >> + >> +/** >> + * em_dev_unregister_perf_domain() - Unregister Energy Model (EM) for a device >> + * @dev : Device for which the EM is registered >> + * >> + * Try to unregister the EM for the specified device (but not a CPU). >> + */ >> +void em_dev_unregister_perf_domain(struct device *dev) >> +{ >> + if (IS_ERR_OR_NULL(dev) || !dev->em_pd) >> + return; >> + >> + if (_is_cpu_device(dev)) >> + return; >> + >> + mutex_lock(&em_pd_mutex); > > Is the mutex really needed?
I just wanted to align this unregister code with register. Since there is debugfs dir lookup and the device's EM existence checks I thought it wouldn't harm just to lock for a while and make sure the registration path is not used. These two paths shouldn't affect each other, but with modules loading/unloading I wanted to play safe. I can change it maybe to just dmb() and the end of the function if it's a big performance problem in this unloading path. What do you think?
> > If this function is called that means there is no more user of the > em_pd, no?
True, that EM users should already be unregistered i.e. thermal cooling.
> >> + em_debug_remove_pd(dev); >> + >> + kfree(dev->em_pd->table); >> + kfree(dev->em_pd); >> + dev->em_pd = NULL; >> + mutex_unlock(&em_pd_mutex); >> +} >> +EXPORT_SYMBOL_GPL(em_dev_unregister_perf_domain); >> > >
Thank you for reviewing this.
Regards, Lukasz
| |