From 7284a00f17290114d304528fc49aeee5c2b9b433 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Tue, 9 Dec 2014 11:18:51 +0530 Subject: PM / OPP: set new_opp->dev_opp to a valid dev_opp We find/allocate dev_opp after using its value to fill new_opp->dev_opp right now. Move this to a later point where dev_opp is valid. Fixes: a7470db6fec4 (PM / OPP don't match for existing OPPs when list is empty) Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 2d195f3a1998..c2020520b2bf 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -400,7 +400,6 @@ static int dev_pm_opp_add_dynamic(struct device *dev, unsigned long freq, mutex_lock(&dev_opp_list_lock); /* populate the opp table */ - new_opp->dev_opp = dev_opp; new_opp->rate = freq; new_opp->u_volt = u_volt; new_opp->available = true; @@ -460,6 +459,7 @@ static int dev_pm_opp_add_dynamic(struct device *dev, unsigned long freq, } list_add: + new_opp->dev_opp = dev_opp; list_add_rcu(&new_opp->node, head); mutex_unlock(&dev_opp_list_lock); -- cgit v1.2.3 From 2a6127d037de96e8add0b09e0200b331a4db54be Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Mon, 8 Dec 2014 13:46:18 +0530 Subject: PM / OPP: remove double calls to find_device_opp() By mistake we called find_device_opp() twice in of_free_opp_table(), fix it. Generated diff doesn't show the problem well and so here is the code snippet: void of_free_opp_table(struct device *dev) { struct device_opp *dev_opp = find_device_opp(dev); struct dev_pm_opp *opp, *tmp; /* Check for existing list for 'dev' */ dev_opp = find_device_opp(dev); ... } Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index c2020520b2bf..1bbef8e838e7 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -768,7 +768,7 @@ EXPORT_SYMBOL_GPL(of_init_opp_table); */ void of_free_opp_table(struct device *dev) { - struct device_opp *dev_opp = find_device_opp(dev); + struct device_opp *dev_opp; struct dev_pm_opp *opp, *tmp; /* Check for existing list for 'dev' */ -- cgit v1.2.3 From 1c6a662f898ecd1615d25fecb8098ea646720a7a Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 10 Dec 2014 09:45:31 +0530 Subject: PM / OPP: replace kfree with kfree_rcu while freeing 'struct device_opp' Somehow one of the instance of freeing resources failed to use kfree_rcu() and used kfree() instead. This might cause problems as the node might be referenced by readers under rcu locks and we must wait for the rcu grace period as well. While we are at it, also update comment over 'struct device_opp' to mention why we are waiting for both rcu and srcu grace periods. Fixes: 129eec55df6a (PM / OPP Introduce APIs to remove OPPs) Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 1bbef8e838e7..e1807268cbf2 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -84,7 +84,11 @@ struct dev_pm_opp { * * This is an internal data structure maintaining the link to opps attached to * a device. This structure is not meant to be shared to users as it is - * meant for book keeping and private to OPP library + * meant for book keeping and private to OPP library. + * + * Because the opp structures can be used from both rcu and srcu readers, we + * need to wait for the grace period of both of them before freeing any + * resources. And so we have used kfree_rcu() from within call_srcu() handlers. */ struct device_opp { struct list_head node; @@ -511,7 +515,7 @@ static void kfree_device_rcu(struct rcu_head *head) { struct device_opp *device_opp = container_of(head, struct device_opp, rcu_head); - kfree(device_opp); + kfree_rcu(device_opp, rcu_head); } void __dev_pm_opp_remove(struct device_opp *dev_opp, struct dev_pm_opp *opp) -- cgit v1.2.3 From 86453b473b1f68c238a6901b26158b4ca3b369bc Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 10 Dec 2014 09:45:32 +0530 Subject: PM / OPP: Staticize __dev_pm_opp_remove() Its a local routine and need not be accessible outside of opp.c. Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index e1807268cbf2..fa065d6e1731 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -518,7 +518,8 @@ static void kfree_device_rcu(struct rcu_head *head) kfree_rcu(device_opp, rcu_head); } -void __dev_pm_opp_remove(struct device_opp *dev_opp, struct dev_pm_opp *opp) +static void __dev_pm_opp_remove(struct device_opp *dev_opp, + struct dev_pm_opp *opp) { /* * Notify the changes in the availability of the operable -- cgit v1.2.3 From 29df0ee1b14ab5cdc83c225258f42600825f45b2 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 10 Dec 2014 09:45:33 +0530 Subject: PM / OPP: reuse find_device_opp() instead of duplicating code Reuse find_device_opp() in opp_set_availability() instead of duplicating code. Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index fa065d6e1731..525ffb202d77 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -597,7 +597,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_remove); static int opp_set_availability(struct device *dev, unsigned long freq, bool availability_req) { - struct device_opp *tmp_dev_opp, *dev_opp = ERR_PTR(-ENODEV); + struct device_opp *dev_opp; struct dev_pm_opp *new_opp, *tmp_opp, *opp = ERR_PTR(-ENODEV); int r = 0; @@ -611,12 +611,7 @@ static int opp_set_availability(struct device *dev, unsigned long freq, mutex_lock(&dev_opp_list_lock); /* Find the device_opp */ - list_for_each_entry(tmp_dev_opp, &dev_opp_list, node) { - if (dev == tmp_dev_opp->dev) { - dev_opp = tmp_dev_opp; - break; - } - } + dev_opp = find_device_opp(dev); if (IS_ERR(dev_opp)) { r = PTR_ERR(dev_opp); dev_warn(dev, "%s: Device OPP not found (%d)\n", __func__, r); -- cgit v1.2.3 From 07cce74a7b259cb90029530e9549bf1d9a1b1c34 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 10 Dec 2014 09:45:34 +0530 Subject: PM / OPP: handle allocation of device_opp in a separate routine Get the 'device_opp' allocation code into a separate routine to keep only the necessary part in dev_pm_opp_add_dynamic(). Also do s/sizeof(struct device_opp)/sizeof(*dev_opp) and remove the print message on kzalloc() failure as checkpatch warns for that. Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 525ffb202d77..1150b9d2e012 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -386,6 +386,27 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, } EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor); +static struct device_opp *add_device_opp(struct device *dev) +{ + struct device_opp *dev_opp; + + /* + * Allocate a new device OPP table. In the infrequent case where a new + * device is needed to be added, we pay this penalty. + */ + dev_opp = kzalloc(sizeof(*dev_opp), GFP_KERNEL); + if (!dev_opp) + return NULL; + + dev_opp->dev = dev; + srcu_init_notifier_head(&dev_opp->srcu_head); + INIT_LIST_HEAD(&dev_opp->opp_list); + + /* Secure the device list modification */ + list_add_rcu(&dev_opp->node, &dev_opp_list); + return dev_opp; +} + static int dev_pm_opp_add_dynamic(struct device *dev, unsigned long freq, unsigned long u_volt, bool dynamic) { @@ -412,27 +433,13 @@ static int dev_pm_opp_add_dynamic(struct device *dev, unsigned long freq, /* Check for existing list for 'dev' */ dev_opp = find_device_opp(dev); if (IS_ERR(dev_opp)) { - /* - * Allocate a new device OPP table. In the infrequent case - * where a new device is needed to be added, we pay this - * penalty. - */ - dev_opp = kzalloc(sizeof(struct device_opp), GFP_KERNEL); + dev_opp = add_device_opp(dev); if (!dev_opp) { mutex_unlock(&dev_opp_list_lock); kfree(new_opp); - dev_warn(dev, - "%s: Unable to create device OPP structure\n", - __func__); return -ENOMEM; } - dev_opp->dev = dev; - srcu_init_notifier_head(&dev_opp->srcu_head); - INIT_LIST_HEAD(&dev_opp->opp_list); - - /* Secure the device list modification */ - list_add_rcu(&dev_opp->node, &dev_opp_list); head = &dev_opp->opp_list; goto list_add; } -- cgit v1.2.3 From 6ce4184d0308888dd6ac2b6ab5f8ec0b2006092e Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 10 Dec 2014 09:45:35 +0530 Subject: PM / OPP: do error handling at the bottom of dev_pm_opp_add_dynamic() This makes it less error prone and moves common resource deallocation at a single place. Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/base/power/opp.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c index 1150b9d2e012..d24dd614a0bd 100644 --- a/drivers/base/power/opp.c +++ b/drivers/base/power/opp.c @@ -413,6 +413,7 @@ static int dev_pm_opp_add_dynamic(struct device *dev, unsigned long freq, struct device_opp *dev_opp = NULL; struct dev_pm_opp *opp, *new_opp; struct list_head *head; + int ret; /* allocate new OPP node */ new_opp = kzalloc(sizeof(*new_opp), GFP_KERNEL); @@ -435,9 +436,8 @@ static int dev_pm_opp_add_dynamic(struct device *dev, unsigned long freq, if (IS_ERR(dev_opp)) { dev_opp = add_device_opp(dev); if (!dev_opp) { - mutex_unlock(&dev_opp_list_lock); - kfree(new_opp); - return -ENOMEM; + ret = -ENOMEM; + goto free_opp; } head = &dev_opp->opp_list; @@ -458,15 +458,13 @@ static int dev_pm_opp_add_dynamic(struct device *dev, unsigned long freq, /* Duplicate OPPs ? */ if (new_opp->rate == opp->rate) { - int ret = opp->available && new_opp->u_volt == opp->u_volt ? + ret = opp->available && new_opp->u_volt == opp->u_volt ? 0 : -EEXIST; dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n", __func__, opp->rate, opp->u_volt, opp->available, new_opp->rate, new_opp->u_volt, new_opp->available); - mutex_unlock(&dev_opp_list_lock); - kfree(new_opp); - return ret; + goto free_opp; } list_add: @@ -480,6 +478,11 @@ list_add: */ srcu_notifier_call_chain(&dev_opp->srcu_head, OPP_EVENT_ADD, new_opp); return 0; + +free_opp: + mutex_unlock(&dev_opp_list_lock); + kfree(new_opp); + return ret; } /** -- cgit v1.2.3