From patchwork Mon Dec 15 23:09:22 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 5498261 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id F07B99F1CD for ; Mon, 15 Dec 2014 23:14:03 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0CD4620A14 for ; Mon, 15 Dec 2014 23:14:03 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 03DDF20A01 for ; Mon, 15 Dec 2014 23:14:02 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Y0eoA-0003GX-55; Mon, 15 Dec 2014 23:12:06 +0000 Received: from pandora.arm.linux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Y0eo4-0001np-Ri for linux-arm-kernel@lists.infradead.org; Mon, 15 Dec 2014 23:12:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=ja4NLiolhj5+2Jb89QiPin/9FqCvsZaq9kLr3OX47hg=; b=p68otBGBilX4Ti6zeuvOKEZP5payinTtamj0L6Q1NwAW8wpmo53qJzrgb8crjemmHVq/aJtrrLBgSsk/qvQBf0mlg6dKlCEx7wt3ir5XQ/VoP4iCmJhR1TUl5rS+/NPwvQSo278i0HNu/4vQUixyVcvukOTD9RwF+YVfUWpAAdM=; Received: from n2100.arm.linux.org.uk ([fd8f:7570:feb6:1:214:fdff:fe10:4f86]:48395) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1Y0ela-0003jG-FZ; Mon, 15 Dec 2014 23:09:26 +0000 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1Y0elX-0003Tq-4P; Mon, 15 Dec 2014 23:09:23 +0000 Date: Mon, 15 Dec 2014 23:09:22 +0000 From: Russell King - ARM Linux To: "Rafael J. Wysocki" Subject: Re: 3.18: lockdep problems in cpufreq Message-ID: <20141215230922.GL11285@n2100.arm.linux.org.uk> References: <20141214213655.GA11285@n2100.arm.linux.org.uk> <003501d01876$fbc53f80$f34fbe80$%brar@samsung.com> <20141215174336.GJ11285@n2100.arm.linux.org.uk> <3353119.0CuA8fKaup@vostro.rjw.lan> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <3353119.0CuA8fKaup@vostro.rjw.lan> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20141215_151201_296931_D89D2F88 X-CRM114-Status: GOOD ( 19.58 ) X-Spam-Score: -0.1 (/) Cc: 'Eduardo Valentin' , 'Viresh Kumar' , Yadwinder Singh Brar , linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Dec 15, 2014 at 10:41:02PM +0100, Rafael J. Wysocki wrote: > On Monday, December 15, 2014 05:43:36 PM Russell King - ARM Linux wrote: > > On Mon, Dec 15, 2014 at 08:24:05PM +0530, Yadwinder Singh Brar wrote: > > > I agree with this approach, if its fine for others also, I can implement > > > and post patch. > > > > Yes, please do. > > Indeed. We have a regression here to fix. Well, here's a patch which I'm running on top of 3.18 at the moment, which is basically what I described in my email, and I'm running with it and it is without any lockdep complaint. 8<=== From: Russell King thermal: cpu_cooling: fix lockdep problems in cpu_cooling A recent change to the cpu_cooling code introduced a AB-BA deadlock scenario between the cpufreq_policy_notifier_list rwsem and the cooling_cpufreq_lock. This is caused by cooling_cpufreq_lock being held before the registration/removal of the notifier block (an operation which takes the rwsem), and the notifier code itself which takes the locks in the reverse order. Solve this by moving to finer grained locking - use one mutex to protect the cpufreq_dev_list as a whole, and a separate lock to ensure correct ordering of cpufreq notifier registration and removal. I considered taking the cooling_list_lock within cooling_cpufreq_lock to protect the registration sequence as a whole, but that adds a dependency between these two locks which is best avoided (lest someone tries to take those two new locks in the reverse order.) In any case, it's safer to have an empty cpufreq_dev_list than to have unnecessary dependencies between locks. Fixes: 2dcd851fe4b4 ("thermal: cpu_cooling: Update always cpufreq policy with thermal constraints") Signed-off-by: Russell King Reviewed-by: Viresh Kumar --- drivers/thermal/cpu_cooling.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index ad09e51ffae4..9e42c6f30785 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -57,6 +57,7 @@ static DEFINE_MUTEX(cooling_cpufreq_lock); static unsigned int cpufreq_dev_count; +static DEFINE_MUTEX(cooling_list_lock); static LIST_HEAD(cpufreq_dev_list); /** @@ -317,7 +318,7 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, if (event != CPUFREQ_ADJUST) return 0; - mutex_lock(&cooling_cpufreq_lock); + mutex_lock(&cooling_list_lock); list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) { if (!cpumask_test_cpu(policy->cpu, &cpufreq_dev->allowed_cpus)) @@ -333,7 +334,7 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, if (policy->max != max_freq) cpufreq_verify_within_limits(policy, 0, max_freq); } - mutex_unlock(&cooling_cpufreq_lock); + mutex_unlock(&cooling_list_lock); return 0; } @@ -482,6 +483,11 @@ __cpufreq_cooling_register(struct device_node *np, } cpufreq_dev->cool_dev = cool_dev; cpufreq_dev->cpufreq_state = 0; + + mutex_lock(&cooling_list_lock); + list_add(&cpufreq_dev->node, &cpufreq_dev_list); + mutex_unlock(&cooling_list_lock); + mutex_lock(&cooling_cpufreq_lock); /* Register the notifier for first cpufreq cooling device */ @@ -489,7 +495,6 @@ __cpufreq_cooling_register(struct device_node *np, cpufreq_register_notifier(&thermal_cpufreq_notifier_block, CPUFREQ_POLICY_NOTIFIER); cpufreq_dev_count++; - list_add(&cpufreq_dev->node, &cpufreq_dev_list); mutex_unlock(&cooling_cpufreq_lock); @@ -553,7 +558,6 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) cpufreq_dev = cdev->devdata; mutex_lock(&cooling_cpufreq_lock); - list_del(&cpufreq_dev->node); cpufreq_dev_count--; /* Unregister the notifier for the last cpufreq cooling device */ @@ -562,6 +566,10 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) CPUFREQ_POLICY_NOTIFIER); mutex_unlock(&cooling_cpufreq_lock); + mutex_lock(&cooling_list_lock); + list_del(&cpufreq_dev->node); + mutex_unlock(&cooling_list_lock); + thermal_cooling_device_unregister(cpufreq_dev->cool_dev); release_idr(&cpufreq_idr, cpufreq_dev->id); kfree(cpufreq_dev);