From patchwork Wed Aug 28 02:57:21 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 2850513 Return-Path: X-Original-To: patchwork-linux-pm@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 B3D249F271 for ; Wed, 28 Aug 2013 02:57:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9D67F20379 for ; Wed, 28 Aug 2013 02:57:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8AB7820377 for ; Wed, 28 Aug 2013 02:57:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752565Ab3H1C5Y (ORCPT ); Tue, 27 Aug 2013 22:57:24 -0400 Received: from smtp.codeaurora.org ([198.145.11.231]:37707 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752481Ab3H1C5X convert rfc822-to-8bit (ORCPT ); Tue, 27 Aug 2013 22:57:23 -0400 Received: from smtp.codeaurora.org (localhost [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id DB5D513F86C; Wed, 28 Aug 2013 02:57:22 +0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 486) id CEED513F86E; Wed, 28 Aug 2013 02:57:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from localhost (i-global252.qualcomm.com [199.106.103.252]) (using SSLv3 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: sboyd@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 5CD9413F86C; Wed, 28 Aug 2013 02:57:22 +0000 (UTC) Date: Tue, 27 Aug 2013 19:57:21 -0700 From: Stephen Boyd To: Viresh Kumar , "Rafael J . Wysocki" Cc: linux-kernel@vger.kernel.org, cpufreq@vger.kernel.org, linux-pm@vger.kernel.org Subject: mutex warning in cpufreq + RFC patch Message-ID: <20130828025721.GA19754@codeaurora.org> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP I'm running this simple test code in a shell on my 3.10 kernel and running into this warning rather quickly. cd /sys/devices/system/cpu/cpu1 while true do echo 0 > online echo 1 > online done & while true do echo 300000 > cpufreq/scaling_min_freq echo 1000000 > cpufreq/scaling_min_freq done (Note you should place valid values for min/max freq in the example above.) WARNING: at kernel/mutex.c:341 __mutex_lock_slowpath+0x14c/0x410() DEBUG_LOCKS_WARN_ON(l->magic != l) Modules linked in: CPU: 0 PID: 1960 Comm: sh Tainted: G W 3.10.0 #32 [] (unwind_backtrace+0x0/0x11c) from [] (show_stack+0x10/0x14) [] (show_stack+0x10/0x14) from [] (warn_slowpath_common+0x4c/0x6c) [] (warn_slowpath_common+0x4c/0x6c) from [] (warn_slowpath_fmt+0x2c/0x3c) [] (warn_slowpath_fmt+0x2c/0x3c) from [] (__mutex_lock_slowpath+0x14c/0x410) [] (__mutex_lock_slowpath+0x14c/0x410) from [] (mutex_lock+0x20/0x3c) [] (mutex_lock+0x20/ 0x3c) from [] (cpufreq_governor_dbs+0x568/0x5f8) [] (cpufreq_governor_dbs+0x568/0x5f8) from [] (__cpufreq_governor+0xdc/0x1a4) [] (__cpufreq_governor+0xdc/0x1a4) from [] (__cpufreq_set_policy+0x278/0x2c0) [] (__cpufreq_set_policy+0x278/0x2c0) from [] (store_scaling_min_freq+0x80/0x9c) [] (store_scaling_min_freq+0x80/0x9c) from [] (store+0x58/0x90) [] (store+0x58/0x90) from [] (sysfs_write_file+0x100/0x148) [] (sysfs_write_file+0x100/0x148) from [] (vfs_write+0xcc/0x174) [] (vfs_write+0xcc/0x174) from [] (SyS_write+0x38/0x64) [] (SyS_write+0x38/0x64) from [] (ret_fast_syscall+0x0/0x30) This is happening because the governor is stopped via hotplug and while we're in the middle of touching the scaling_min_freq file. When the governor is stopped we destroy the timer_mutex that the scaling_min_freq thread is just about to acquire. From what I can tell, we shouldn't be stopping the governor until after the kobjects go away or we should start and stop the governor while holding the policy semaphore otherwise userspace can come in and use uninitialized things. I have this hack which seems to mostly work. Thoughts? ----8<---- diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index cbfe3c1..134004b 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -823,11 +823,11 @@ static int cpufreq_add_policy_cpu(unsigned int cpu, unsigned int sibling, policy = cpufreq_cpu_get(sibling); WARN_ON(!policy); + lock_policy_rwsem_write(sibling); + if (has_target) __cpufreq_governor(policy, CPUFREQ_GOV_STOP); - lock_policy_rwsem_write(sibling); - write_lock_irqsave(&cpufreq_driver_lock, flags); cpumask_set_cpu(cpu, policy->cpus); @@ -835,12 +835,11 @@ static int cpufreq_add_policy_cpu(unsigned int cpu, unsigned int sibling, per_cpu(cpufreq_cpu_data, cpu) = policy; write_unlock_irqrestore(&cpufreq_driver_lock, flags); - unlock_policy_rwsem_write(sibling); - if (has_target) { __cpufreq_governor(policy, CPUFREQ_GOV_START); __cpufreq_governor(policy, CPUFREQ_GOV_LIMITS); } + unlock_policy_rwsem_write(sibling); ret = sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq"); if (ret) { @@ -1037,9 +1036,6 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif return -EINVAL; } - if (cpufreq_driver->target) - __cpufreq_governor(data, CPUFREQ_GOV_STOP); - #ifdef CONFIG_HOTPLUG_CPU if (!cpufreq_driver->setpolicy) strncpy(per_cpu(cpufreq_cpu_governor, cpu), @@ -1048,9 +1044,6 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif WARN_ON(lock_policy_rwsem_write(cpu)); cpus = cpumask_weight(data->cpus); - - if (cpus > 1) - cpumask_clear_cpu(cpu, data->cpus); unlock_policy_rwsem_write(cpu); if (cpu != data->cpu) { @@ -1086,9 +1079,6 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif /* If cpu is last user of policy, free policy */ if (cpus == 1) { - if (cpufreq_driver->target) - __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT); - lock_policy_rwsem_read(cpu); kobj = &data->kobj; cmp = &data->kobj_unregister; @@ -1103,6 +1093,11 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif wait_for_completion(cmp); pr_debug("wait complete\n"); + if (cpufreq_driver->target) { + __cpufreq_governor(data, CPUFREQ_GOV_STOP); + __cpufreq_governor(data, CPUFREQ_GOV_POLICY_EXIT); + } + if (cpufreq_driver->exit) cpufreq_driver->exit(data); @@ -1113,8 +1108,13 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif pr_debug("%s: removing link, cpu: %d\n", __func__, cpu); cpufreq_cpu_put(data); if (cpufreq_driver->target) { + WARN_ON(lock_policy_rwsem_write(cpu)); + __cpufreq_governor(data, CPUFREQ_GOV_STOP); + if (cpus > 1) + cpumask_clear_cpu(cpu, data->cpus); __cpufreq_governor(data, CPUFREQ_GOV_START); __cpufreq_governor(data, CPUFREQ_GOV_LIMITS); + unlock_policy_rwsem_write(cpu); } }