From patchwork Mon Jan 23 04:41:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 92186 Delivered-To: patch@linaro.org Received: by 10.182.3.34 with SMTP id 2csp1055230obz; Sun, 22 Jan 2017 20:42:40 -0800 (PST) X-Received: by 10.84.209.204 with SMTP id y70mr40184485plh.180.1485146560457; Sun, 22 Jan 2017 20:42:40 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id s185si14404172pgc.259.2017.01.22.20.42.40; Sun, 22 Jan 2017 20:42:40 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-pm-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751053AbdAWEmi (ORCPT + 13 others); Sun, 22 Jan 2017 23:42:38 -0500 Received: from mail-pf0-f173.google.com ([209.85.192.173]:34687 "EHLO mail-pf0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751105AbdAWEmf (ORCPT ); Sun, 22 Jan 2017 23:42:35 -0500 Received: by mail-pf0-f173.google.com with SMTP id e4so37806757pfg.1 for ; Sun, 22 Jan 2017 20:42:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=khmZmFD+KDsOz5MR6SAABcPX1kbN+Wj/vodmR3Tceys=; b=dXu1UTTxtaZmljrkhtX0XqP2EmFi7WJTJgNTqnuFRLNtd7ChaD7VPjUmBMx0pi3mSF GrdvznlkdXkAv8KKfDY7vlPgJWsoREkDSgflcaP9Iq8tyIR/pu8kAl7EIQfIyFcluWgM DkZoMP/wM3SFJQ0AAv74nQrjL2xT708muVkIw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=khmZmFD+KDsOz5MR6SAABcPX1kbN+Wj/vodmR3Tceys=; b=Rs+/GtXz0cpx0s1TL++7HsQPmiJQxbr1H9Jn5fasDoDO/twg+aiN5c98uhrLU4fAYJ GsiloywTaElRbweXXF4tHD+xD1k2n8xkrusE4U1hOCcdQgD0+6/eixHSARgmUgQxJdQI eDMw/we/+ojB6qQ8mNn1m7xUsvuq67jOvU9Dkyo8VOPwBjE+BAbZstr3/HvyATO51UQe nqqEL6+SH7H0zz4dKP1l8onnaGli6rFn6ZgqocLxGTaOoO5MuQ8iHdM7yH9CoVlckkr5 A9lt6XbGpGl0jGLfzSPJhNj/ljiiPWpxyA5FRf6PJolljSpDJqw2RfSYLP08uU6ltGiR HkbQ== X-Gm-Message-State: AIkVDXKtXQ1yt8Kno5/zEoL5+dUgP2zgyQBRC218LHiBaYof4ph1Ze0CTD8WTM58KPcPXTtR X-Received: by 10.84.248.10 with SMTP id p10mr39807133pll.87.1485146549702; Sun, 22 Jan 2017 20:42:29 -0800 (PST) Received: from localhost ([122.171.65.82]) by smtp.gmail.com with ESMTPSA id t21sm32739171pfa.1.2017.01.22.20.42.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 22 Jan 2017 20:42:29 -0800 (PST) From: Viresh Kumar To: Rafael Wysocki , Viresh Kumar , Nishanth Menon , Stephen Boyd Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Vincent Guittot , Viresh Kumar Subject: [PATCH V2 08/12] PM / OPP: Take kref from _find_opp_table() Date: Mon, 23 Jan 2017 10:11:48 +0530 Message-Id: <94b7282841667edfeefdd1c38f65375505fb3f8a.1485146406.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.7.1.410.g6faf27b In-Reply-To: References: In-Reply-To: References: Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org Take reference of the OPP table from within _find_opp_table(). Also update the callers of _find_opp_table() to call dev_pm_opp_put_opp_table() after they have used the OPP table. Note that _find_opp_table() increments the reference under the opp_table_lock. Now that the OPP table wouldn't get freed until the callers of _find_opp_table() call dev_pm_opp_put_opp_table(), there is no need to take the opp_table_lock or rcu_read_lock() around it. Drop them. Signed-off-by: Viresh Kumar Reviewed-by: Stephen Boyd --- drivers/base/power/opp/core.c | 191 +++++++++++++++++++----------------------- drivers/base/power/opp/cpu.c | 26 ++---- 2 files changed, 95 insertions(+), 122 deletions(-) -- 2.7.1.410.g6faf27b -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index a6efa818029a..69452a4bcda7 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -54,6 +54,21 @@ static struct opp_device *_find_opp_dev(const struct device *dev, return NULL; } +struct opp_table *_find_opp_table_unlocked(struct device *dev) +{ + struct opp_table *opp_table; + + list_for_each_entry(opp_table, &opp_tables, node) { + if (_find_opp_dev(dev, opp_table)) { + _get_opp_table_kref(opp_table); + + return opp_table; + } + } + + return ERR_PTR(-ENODEV); +} + /** * _find_opp_table() - find opp_table struct using device pointer * @dev: device pointer used to lookup OPP table @@ -64,28 +79,22 @@ static struct opp_device *_find_opp_dev(const struct device *dev, * Return: pointer to 'struct opp_table' if found, otherwise -ENODEV or * -EINVAL based on type of error. * - * Locking: For readers, this function must be called under rcu_read_lock(). - * opp_table is a RCU protected pointer, which means that opp_table is valid - * as long as we are under RCU lock. - * - * For Writers, this function must be called with opp_table_lock held. + * The callers must call dev_pm_opp_put_opp_table() after the table is used. */ struct opp_table *_find_opp_table(struct device *dev) { struct opp_table *opp_table; - opp_rcu_lockdep_assert(); - if (IS_ERR_OR_NULL(dev)) { pr_err("%s: Invalid parameters\n", __func__); return ERR_PTR(-EINVAL); } - list_for_each_entry_rcu(opp_table, &opp_tables, node) - if (_find_opp_dev(dev, opp_table)) - return opp_table; + mutex_lock(&opp_table_lock); + opp_table = _find_opp_table_unlocked(dev); + mutex_unlock(&opp_table_lock); - return ERR_PTR(-ENODEV); + return opp_table; } /** @@ -175,23 +184,20 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_is_turbo); * @dev: device for which we do this operation * * Return: This function returns the max clock latency in nanoseconds. - * - * Locking: This function takes rcu_read_lock(). */ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev) { struct opp_table *opp_table; unsigned long clock_latency_ns; - rcu_read_lock(); - opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) - clock_latency_ns = 0; - else - clock_latency_ns = opp_table->clock_latency_ns_max; + return 0; + + clock_latency_ns = opp_table->clock_latency_ns_max; + + dev_pm_opp_put_opp_table(opp_table); - rcu_read_unlock(); return clock_latency_ns; } EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency); @@ -201,15 +207,13 @@ static int _get_regulator_count(struct device *dev) struct opp_table *opp_table; int count; - rcu_read_lock(); - opp_table = _find_opp_table(dev); - if (!IS_ERR(opp_table)) - count = opp_table->regulator_count; - else - count = 0; + if (IS_ERR(opp_table)) + return 0; - rcu_read_unlock(); + count = opp_table->regulator_count; + + dev_pm_opp_put_opp_table(opp_table); return count; } @@ -248,13 +252,11 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) if (!uV) goto free_regulators; - rcu_read_lock(); - opp_table = _find_opp_table(dev); - if (IS_ERR(opp_table)) { - rcu_read_unlock(); + if (IS_ERR(opp_table)) goto free_uV; - } + + rcu_read_lock(); memcpy(regulators, opp_table->regulators, count * sizeof(*regulators)); @@ -274,6 +276,7 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) } rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); /* * The caller needs to ensure that opp_table (and hence the regulator) @@ -323,17 +326,15 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device *dev) struct opp_table *opp_table; unsigned long freq = 0; - rcu_read_lock(); - opp_table = _find_opp_table(dev); - if (IS_ERR(opp_table) || !opp_table->suspend_opp || - !opp_table->suspend_opp->available) - goto unlock; + if (IS_ERR(opp_table)) + return 0; - freq = dev_pm_opp_get_freq(opp_table->suspend_opp); + if (opp_table->suspend_opp && opp_table->suspend_opp->available) + freq = dev_pm_opp_get_freq(opp_table->suspend_opp); + + dev_pm_opp_put_opp_table(opp_table); -unlock: - rcu_read_unlock(); return freq; } EXPORT_SYMBOL_GPL(dev_pm_opp_get_suspend_opp_freq); @@ -353,23 +354,24 @@ int dev_pm_opp_get_opp_count(struct device *dev) struct dev_pm_opp *temp_opp; int count = 0; - rcu_read_lock(); - opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { count = PTR_ERR(opp_table); dev_err(dev, "%s: OPP table not found (%d)\n", __func__, count); - goto out_unlock; + return count; } + rcu_read_lock(); + list_for_each_entry_rcu(temp_opp, &opp_table->opp_list, node) { if (temp_opp->available) count++; } -out_unlock: rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); + return count; } EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count); @@ -404,17 +406,16 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev, struct opp_table *opp_table; struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); - rcu_read_lock(); - opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { int r = PTR_ERR(opp_table); dev_err(dev, "%s: OPP table not found (%d)\n", __func__, r); - rcu_read_unlock(); return ERR_PTR(r); } + rcu_read_lock(); + list_for_each_entry_rcu(temp_opp, &opp_table->opp_list, node) { if (temp_opp->available == available && temp_opp->rate == freq) { @@ -427,6 +428,7 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev, } rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); return opp; } @@ -480,17 +482,16 @@ struct dev_pm_opp *dev_pm_opp_find_freq_ceil(struct device *dev, return ERR_PTR(-EINVAL); } - rcu_read_lock(); - opp_table = _find_opp_table(dev); - if (IS_ERR(opp_table)) { - rcu_read_unlock(); + if (IS_ERR(opp_table)) return ERR_CAST(opp_table); - } + + rcu_read_lock(); opp = _find_freq_ceil(opp_table, freq); rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); return opp; } @@ -525,13 +526,11 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, return ERR_PTR(-EINVAL); } - rcu_read_lock(); - opp_table = _find_opp_table(dev); - if (IS_ERR(opp_table)) { - rcu_read_unlock(); + if (IS_ERR(opp_table)) return ERR_CAST(opp_table); - } + + rcu_read_lock(); list_for_each_entry_rcu(temp_opp, &opp_table->opp_list, node) { if (temp_opp->available) { @@ -547,6 +546,7 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, if (!IS_ERR(opp)) dev_pm_opp_get(opp); rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); if (!IS_ERR(opp)) *freq = opp->rate; @@ -564,22 +564,18 @@ static struct clk *_get_opp_clk(struct device *dev) struct opp_table *opp_table; struct clk *clk; - rcu_read_lock(); - opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { dev_err(dev, "%s: device opp doesn't exist\n", __func__); - clk = ERR_CAST(opp_table); - goto unlock; + return ERR_CAST(opp_table); } clk = opp_table->clk; if (IS_ERR(clk)) dev_err(dev, "%s: No clock available for the device\n", __func__); + dev_pm_opp_put_opp_table(opp_table); -unlock: - rcu_read_unlock(); return clk; } @@ -715,15 +711,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) return 0; } - rcu_read_lock(); - opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { dev_err(dev, "%s: device opp doesn't exist\n", __func__); - rcu_read_unlock(); return PTR_ERR(opp_table); } + rcu_read_lock(); + old_opp = _find_freq_ceil(opp_table, &old_freq); if (IS_ERR(old_opp)) { dev_err(dev, "%s: failed to find current OPP for freq %lu (%ld)\n", @@ -738,6 +733,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) if (!IS_ERR(old_opp)) dev_pm_opp_put(old_opp); rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); return ret; } @@ -752,6 +748,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) if (!IS_ERR(old_opp)) dev_pm_opp_put(old_opp); rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); return _generic_set_opp_clk_only(dev, clk, old_freq, freq); } @@ -780,6 +777,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) if (!IS_ERR(old_opp)) dev_pm_opp_put(old_opp); rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); return set_opp(data); } @@ -893,11 +891,9 @@ struct opp_table *dev_pm_opp_get_opp_table(struct device *dev) /* Hold our table modification lock here */ mutex_lock(&opp_table_lock); - opp_table = _find_opp_table(dev); - if (!IS_ERR(opp_table)) { - _get_opp_table_kref(opp_table); + opp_table = _find_opp_table_unlocked(dev); + if (!IS_ERR(opp_table)) goto unlock; - } opp_table = _allocate_opp_table(dev); @@ -1004,12 +1000,9 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq) struct opp_table *opp_table; bool found = false; - /* Hold our table modification lock here */ - mutex_lock(&opp_table_lock); - opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) - goto unlock; + return; mutex_lock(&opp_table->lock); @@ -1022,15 +1015,14 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq) mutex_unlock(&opp_table->lock); - if (!found) { + if (found) { + dev_pm_opp_put(opp); + } else { dev_warn(dev, "%s: Couldn't find OPP with freq: %lu\n", __func__, freq); - goto unlock; } - dev_pm_opp_put(opp); -unlock: - mutex_unlock(&opp_table_lock); + dev_pm_opp_put_opp_table(opp_table); } EXPORT_SYMBOL_GPL(dev_pm_opp_remove); @@ -1648,14 +1640,12 @@ static int _opp_set_availability(struct device *dev, unsigned long freq, if (!new_opp) return -ENOMEM; - mutex_lock(&opp_table_lock); - /* Find the opp_table */ opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { r = PTR_ERR(opp_table); dev_warn(dev, "%s: Device OPP not found (%d)\n", __func__, r); - goto unlock; + goto free_opp; } mutex_lock(&opp_table->lock); @@ -1668,8 +1658,6 @@ static int _opp_set_availability(struct device *dev, unsigned long freq, } } - mutex_unlock(&opp_table->lock); - if (IS_ERR(opp)) { r = PTR_ERR(opp); goto unlock; @@ -1685,7 +1673,6 @@ static int _opp_set_availability(struct device *dev, unsigned long freq, new_opp->available = availability_req; list_replace_rcu(&opp->node, &new_opp->node); - mutex_unlock(&opp_table_lock); call_srcu(&opp_table->srcu_head.srcu, &opp->rcu_head, _kfree_opp_rcu); /* Notify the change of the OPP availability */ @@ -1696,10 +1683,14 @@ static int _opp_set_availability(struct device *dev, unsigned long freq, srcu_notifier_call_chain(&opp_table->srcu_head, OPP_EVENT_DISABLE, new_opp); + mutex_unlock(&opp_table->lock); + dev_pm_opp_put_opp_table(opp_table); return 0; unlock: - mutex_unlock(&opp_table_lock); + mutex_unlock(&opp_table->lock); + dev_pm_opp_put_opp_table(opp_table); +free_opp: kfree(new_opp); return r; } @@ -1767,18 +1758,16 @@ int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb) struct opp_table *opp_table; int ret; - rcu_read_lock(); - opp_table = _find_opp_table(dev); - if (IS_ERR(opp_table)) { - ret = PTR_ERR(opp_table); - goto unlock; - } + if (IS_ERR(opp_table)) + return PTR_ERR(opp_table); + + rcu_read_lock(); ret = srcu_notifier_chain_register(&opp_table->srcu_head, nb); -unlock: rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); return ret; } @@ -1797,18 +1786,14 @@ int dev_pm_opp_unregister_notifier(struct device *dev, struct opp_table *opp_table; int ret; - rcu_read_lock(); - opp_table = _find_opp_table(dev); - if (IS_ERR(opp_table)) { - ret = PTR_ERR(opp_table); - goto unlock; - } + if (IS_ERR(opp_table)) + return PTR_ERR(opp_table); ret = srcu_notifier_chain_unregister(&opp_table->srcu_head, nb); -unlock: rcu_read_unlock(); + dev_pm_opp_put_opp_table(opp_table); return ret; } @@ -1839,9 +1824,6 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all) { struct opp_table *opp_table; - /* Hold our table modification lock here */ - mutex_lock(&opp_table_lock); - /* Check for existing table for 'dev' */ opp_table = _find_opp_table(dev); if (IS_ERR(opp_table)) { @@ -1852,13 +1834,12 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev, bool remove_all) IS_ERR_OR_NULL(dev) ? "Invalid device" : dev_name(dev), error); - goto unlock; + return; } _dev_pm_opp_remove_table(opp_table, dev, remove_all); -unlock: - mutex_unlock(&opp_table_lock); + dev_pm_opp_put_opp_table(opp_table); } /** diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c index adef788862d5..df29f08eecc4 100644 --- a/drivers/base/power/opp/cpu.c +++ b/drivers/base/power/opp/cpu.c @@ -174,13 +174,9 @@ int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, struct device *dev; int cpu, ret = 0; - mutex_lock(&opp_table_lock); - opp_table = _find_opp_table(cpu_dev); - if (IS_ERR(opp_table)) { - ret = PTR_ERR(opp_table); - goto unlock; - } + if (IS_ERR(opp_table)) + return PTR_ERR(opp_table); for_each_cpu(cpu, cpumask) { if (cpu == cpu_dev->id) @@ -203,8 +199,8 @@ int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, /* Mark opp-table as multiple CPUs are sharing it now */ opp_table->shared_opp = OPP_TABLE_ACCESS_SHARED; } -unlock: - mutex_unlock(&opp_table_lock); + + dev_pm_opp_put_opp_table(opp_table); return ret; } @@ -232,17 +228,13 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask) struct opp_table *opp_table; int ret = 0; - mutex_lock(&opp_table_lock); - opp_table = _find_opp_table(cpu_dev); - if (IS_ERR(opp_table)) { - ret = PTR_ERR(opp_table); - goto unlock; - } + if (IS_ERR(opp_table)) + return PTR_ERR(opp_table); if (opp_table->shared_opp == OPP_TABLE_ACCESS_UNKNOWN) { ret = -EINVAL; - goto unlock; + goto put_opp_table; } cpumask_clear(cpumask); @@ -254,8 +246,8 @@ int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask) cpumask_set_cpu(cpu_dev->id, cpumask); } -unlock: - mutex_unlock(&opp_table_lock); +put_opp_table: + dev_pm_opp_put_opp_table(opp_table); return ret; }