Re: [PATCH v9 4/8] PM / devfreq: Move more initialization before registration
From: Chanwoo Choi <cw00.choi@samsung.com>
Date: 2019-10-31 03:10:18
Also in:
linux-pm
Hi Leonard, This patch didn't get the acked-by from devfreq maintainer. I think that we need to discuss this patch with more time. Also, it is possible to make it as the separate patch from this series. IMHO, if you make the separate patch for this and resend the separate patch on later, I think that we can merge the remained patch related to PM_QOS. On 19. 10. 3. 오전 4:25, Leonard Crestez wrote:
quoted hunk ↗ jump to hunk
In general it is a better to initialize an object before making it accessible externally (through device_register). This makes it possible to avoid remove locking the partially initialized object and simplifies the code. However devm is not available before device_register (only after the device_initialize step) so the two allocations need to be managed manually. Signed-off-by: Leonard Crestez <redacted> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> --- drivers/devfreq/devfreq.c | 43 +++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 18 deletions(-)diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 3e0e936185a3..0b40f40ee7aa 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c@@ -591,10 +591,12 @@ static void devfreq_dev_release(struct device *dev) mutex_unlock(&devfreq_list_lock); if (devfreq->profile->exit) devfreq->profile->exit(devfreq->dev.parent); + kfree(devfreq->time_in_state); + kfree(devfreq->trans_table); mutex_destroy(&devfreq->lock); kfree(devfreq); } /**@@ -674,44 +676,43 @@ struct devfreq *devfreq_add_device(struct device *dev, devfreq->max_freq = devfreq->scaling_max_freq; devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); atomic_set(&devfreq->suspend_count, 0); - dev_set_name(&devfreq->dev, "devfreq%d", - atomic_inc_return(&devfreq_no)); - err = device_register(&devfreq->dev); - if (err) { - mutex_unlock(&devfreq->lock); - put_device(&devfreq->dev); - goto err_out; - } - - devfreq->trans_table = devm_kzalloc(&devfreq->dev, + devfreq->trans_table = kzalloc( array3_size(sizeof(unsigned int), devfreq->profile->max_state, devfreq->profile->max_state), GFP_KERNEL); if (!devfreq->trans_table) { mutex_unlock(&devfreq->lock); err = -ENOMEM; - goto err_devfreq; + goto err_dev; } - devfreq->time_in_state = devm_kcalloc(&devfreq->dev, - devfreq->profile->max_state, - sizeof(unsigned long), - GFP_KERNEL); + devfreq->time_in_state = kcalloc(devfreq->profile->max_state, + sizeof(unsigned long), + GFP_KERNEL); if (!devfreq->time_in_state) { mutex_unlock(&devfreq->lock); err = -ENOMEM; - goto err_devfreq; + goto err_dev; } devfreq->last_stat_updated = jiffies; srcu_init_notifier_head(&devfreq->transition_notifier_list); + dev_set_name(&devfreq->dev, "devfreq%d", + atomic_inc_return(&devfreq_no)); + err = device_register(&devfreq->dev); + if (err) { + mutex_unlock(&devfreq->lock); + put_device(&devfreq->dev); + goto err_out; + } + mutex_unlock(&devfreq->lock); mutex_lock(&devfreq_list_lock); governor = try_then_request_governor(devfreq->governor_name);@@ -737,14 +738,20 @@ struct devfreq *devfreq_add_device(struct device *dev, return devfreq; err_init: mutex_unlock(&devfreq_list_lock); -err_devfreq: devfreq_remove_device(devfreq); - devfreq = NULL; + return ERR_PTR(err); + err_dev: + /* + * Cleanup path for errors that happen before registration. + * Otherwise we rely on devfreq_dev_release. + */ + kfree(devfreq->time_in_state); + kfree(devfreq->trans_table); kfree(devfreq); err_out: return ERR_PTR(err); } EXPORT_SYMBOL(devfreq_add_device);
-- Best Regards, Chanwoo Choi Samsung Electronics _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel