Re: [PATCH RFC 1/3] PM / Domains: Allocate memory outside domain locks

On Sun, Jun 07 2015 at 02:35 -0600, Krzysztof Kozlowski wrote:

W dniu 05.06.2015 o 07:29, Lina Iyer pisze:

quoted

In order for domains to be powered on/off in irq locked context, the
domain locks could either be a spinlock or a mutex, depending on the
domain. In preparation for atomic support, allocate domain data outside
the domain locks, so the allocation calls dont have to be context
sensitive.

Cc: Ulf Hansson <redacted>
Cc: Rafael J. Wysocki <redacted>
Cc: Kevin Hilman <redacted>
Signed-off-by: Lina Iyer <redacted>
---
 drivers/base/power/domain.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 3b3367b..dfd7595 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c

@@ -1367,13 +1367,17 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 			   struct generic_pm_domain *subdomain)
 {
-	struct gpd_link *link;
+	struct gpd_link *link, *itr;
 	int ret = 0;

 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(subdomain)
 	    || genpd == subdomain)
 		return -EINVAL;

+	link = kzalloc(sizeof(*link), GFP_KERNEL);
+	if (!link)
+		return -ENOMEM;
+
 	mutex_lock(&genpd->lock);
 	mutex_lock_nested(&subdomain->lock, SINGLE_DEPTH_NESTING);

@@ -1383,18 +1387,13 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 		goto out;
 	}

-	list_for_each_entry(link, &genpd->master_links, master_node) {
-		if (link->slave == subdomain && link->master == genpd) {
+	list_for_each_entry(itr, &genpd->master_links, master_node) {
+		if (itr->slave == subdomain && itr->master == genpd) {
 			ret = -EINVAL;
 			goto out;
 		}
 	}

-	link = kzalloc(sizeof(*link), GFP_KERNEL);
-	if (!link) {
-		ret = -ENOMEM;
-		goto out;
-	}
 	link->master = genpd;
 	list_add_tail(&link->master_node, &genpd->master_links);
 	link->slave = subdomain;

@@ -1496,17 +1495,17 @@ int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state)
 	if (IS_ERR_OR_NULL(genpd) || state < 0)
 		return -EINVAL;

+	cpuidle_data = kzalloc(sizeof(*cpuidle_data), GFP_KERNEL);
+	if (!cpuidle_data)
+		return -ENOMEM;
+
 	mutex_lock(&genpd->lock);

 	if (genpd->cpuidle_data) {
 		ret = -EEXIST;
 		goto out;

I think you must update the error paths. In this case the kfree() is
missing.

Yes, thank you. Will fix.

-- Lina

quoted

 	}
-	cpuidle_data = kzalloc(sizeof(*cpuidle_data), GFP_KERNEL);
-	if (!cpuidle_data) {
-		ret = -ENOMEM;
-		goto out;
-	}
+
 	cpuidle_drv = cpuidle_driver_ref();
 	if (!cpuidle_drv) {
 		ret = -ENODEV;