From patchwork Mon Sep 12 06:37:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 75964 Delivered-To: patch@linaro.org Received: by 10.140.106.72 with SMTP id d66csp704077qgf; Sun, 11 Sep 2016 23:37:17 -0700 (PDT) X-Received: by 10.66.190.200 with SMTP id gs8mr30904895pac.42.1473662237144; Sun, 11 Sep 2016 23:37:17 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id q7si20066715pay.47.2016.09.11.23.37.16; Sun, 11 Sep 2016 23:37:17 -0700 (PDT) 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 dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756419AbcILGhN (ORCPT + 14 others); Mon, 12 Sep 2016 02:37:13 -0400 Received: from mail-pf0-f175.google.com ([209.85.192.175]:35750 "EHLO mail-pf0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756313AbcILGhM (ORCPT ); Mon, 12 Sep 2016 02:37:12 -0400 Received: by mail-pf0-f175.google.com with SMTP id i75so9205972pfa.2 for ; Sun, 11 Sep 2016 23:37:12 -0700 (PDT) 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; bh=LJKIR/qSxsxNzxQ+FDd30xCYGPMPugZVzQpurzg/5cA=; b=bhtpqHuW1eEfGk1NJ3RxD4ffA57sB9fUFVCENWBNEWvw9Y+FpgOrPjm5eMuuVKMjH9 fab8jkAajmY9nrbiql1Ao/f1NxvdreJw6QxFKeNyOruZ0gToo0jFc07OXrhOtTMQQ/61 Ap6STlistxZBYskOGtAnidqCEiKQ/Fa7Ko8+8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=LJKIR/qSxsxNzxQ+FDd30xCYGPMPugZVzQpurzg/5cA=; b=ippbziyvKn53bnYBZ3CFW6/bhiAhMAqaXHcGaT4gzB7P0KE0S4cJJoIKDjgp8yAw/m mW0ZYf8hztW9OezCqwbrHWDNKyVrWIzkVG0g1f5U/+NBEqh6oXNmctfG8eRmlinEFoHw 6VbGoRRbRsv0mGoJi/mfLWfQjMMFrOpWENolK9yM/TJ29blzucTntlAq+ywoRMmbx5dJ QyEPBQ05Z+j4ZoGeclIcZYtqD7+v3Ml4VfHbMAUMNqKvqqH+BqWintpOdXZwsZyAbAuv LKyGrIbhD97sX75gcgug3Ab6NGrQLwYQu18GXEFsCl/PNpycWk475+La2y3LmnLh9fDj bgxw== X-Gm-Message-State: AE9vXwMV/n/r9pqBrUPc8ZGVGqPFsesjki/lZLnLcdbuKJRxJm8p8SayyRFHfSZGGDT/J7/3 X-Received: by 10.98.8.13 with SMTP id c13mr30469268pfd.166.1473662231751; Sun, 11 Sep 2016 23:37:11 -0700 (PDT) Received: from localhost ([171.61.121.167]) by smtp.gmail.com with ESMTPSA id e187sm21492915pfa.58.2016.09.11.23.37.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 11 Sep 2016 23:37:10 -0700 (PDT) From: Viresh Kumar To: Rafael Wysocki , Russell King , Viresh Kumar Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Russell King Subject: [PATCH V2] cpufreq: create link to policy only for registered CPUs Date: Mon, 12 Sep 2016 12:07:05 +0530 Message-Id: <59daae7da84077fbf845b1aad2962933585f8e35.1473662181.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.7.1.410.g6faf27b In-Reply-To: <20160819110032.GM1041@n2100.armlinux.org.uk> References: <20160819110032.GM1041@n2100.armlinux.org.uk> Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org If a cpufreq driver is registered very early in the boot stage (e.g. registered from postcore_initcall()), then cpufreq core may generate kernel warnings for it. In this case, the CPUs are brought online, then the cpufreq driver is registered, and then the CPU topology devices are registered. However, by the time cpufreq_add_dev() gets called, the cpu device isn't stored in the per-cpu variable (cpu_sys_devices,) which is read by get_cpu_device(). So the cpufreq core fails to get device for the CPU, for which cpufreq_add_dev() was called in the first place and we will hit a WARN_ON(!cpu_dev). Even if we reuse the 'dev' parameter passed to cpufreq_add_dev() to avoid that warning, there might be other CPUs online that share the policy with the cpu for which cpufreq_add_dev() is called. Eventually get_cpu_device() will return NULL for them as well, and we will hit the same WARN_ON() again. In order to fix these issues, change cpufreq core to create links to the policy for a cpu only when cpufreq_add_dev() is called for that CPU. Reuse the 'real_cpus' mask to track that as well. Note that cpufreq_remove_dev() already handles removal of the links for individual CPUs and cpufreq_add_dev() has aligned with that now. Reported-by: Russell King Tested-by: Russell King Signed-off-by: Viresh Kumar --- V1->V2: - Updated changelog based on suggestions from Russell - Tested by from Russell drivers/cpufreq/cpufreq.c | 89 +++++++++++++++-------------------------------- 1 file changed, 28 insertions(+), 61 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/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 13fb589b6d2c..3a64136bf21b 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -916,58 +916,18 @@ static struct kobj_type ktype_cpufreq = { .release = cpufreq_sysfs_release, }; -static int add_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu) +static int add_cpu_dev_symlink(struct cpufreq_policy *policy, + struct device *dev) { - struct device *cpu_dev; - - pr_debug("%s: Adding symlink for CPU: %u\n", __func__, cpu); - - if (!policy) - return 0; - - cpu_dev = get_cpu_device(cpu); - if (WARN_ON(!cpu_dev)) - return 0; - - return sysfs_create_link(&cpu_dev->kobj, &policy->kobj, "cpufreq"); + dev_dbg(dev, "%s: Adding symlink\n", __func__); + return sysfs_create_link(&dev->kobj, &policy->kobj, "cpufreq"); } -static void remove_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu) +static void remove_cpu_dev_symlink(struct cpufreq_policy *policy, + struct device *dev) { - struct device *cpu_dev; - - pr_debug("%s: Removing symlink for CPU: %u\n", __func__, cpu); - - cpu_dev = get_cpu_device(cpu); - if (WARN_ON(!cpu_dev)) - return; - - sysfs_remove_link(&cpu_dev->kobj, "cpufreq"); -} - -/* Add/remove symlinks for all related CPUs */ -static int cpufreq_add_dev_symlink(struct cpufreq_policy *policy) -{ - unsigned int j; - int ret = 0; - - /* Some related CPUs might not be present (physically hotplugged) */ - for_each_cpu(j, policy->real_cpus) { - ret = add_cpu_dev_symlink(policy, j); - if (ret) - break; - } - - return ret; -} - -static void cpufreq_remove_dev_symlink(struct cpufreq_policy *policy) -{ - unsigned int j; - - /* Some related CPUs might not be present (physically hotplugged) */ - for_each_cpu(j, policy->real_cpus) - remove_cpu_dev_symlink(policy, j); + dev_dbg(dev, "%s: Removing symlink\n", __func__); + sysfs_remove_link(&dev->kobj, "cpufreq"); } static int cpufreq_add_dev_interface(struct cpufreq_policy *policy) @@ -999,7 +959,7 @@ static int cpufreq_add_dev_interface(struct cpufreq_policy *policy) return ret; } - return cpufreq_add_dev_symlink(policy); + return 0; } __weak struct cpufreq_governor *cpufreq_default_governor(void) @@ -1129,7 +1089,6 @@ static void cpufreq_policy_put_kobj(struct cpufreq_policy *policy, bool notify) down_write(&policy->rwsem); cpufreq_stats_free_table(policy); - cpufreq_remove_dev_symlink(policy); kobj = &policy->kobj; cmp = &policy->kobj_unregister; up_write(&policy->rwsem); @@ -1211,8 +1170,8 @@ static int cpufreq_online(unsigned int cpu) if (new_policy) { /* related_cpus should at least include policy->cpus. */ cpumask_copy(policy->related_cpus, policy->cpus); - /* Remember CPUs present at the policy creation time. */ - cpumask_and(policy->real_cpus, policy->cpus, cpu_present_mask); + /* Clear mask of registered CPUs */ + cpumask_clear(policy->real_cpus); } /* @@ -1327,6 +1286,8 @@ static int cpufreq_online(unsigned int cpu) return ret; } +static void cpufreq_offline(unsigned int cpu); + /** * cpufreq_add_dev - the cpufreq interface for a CPU device. * @dev: CPU device. @@ -1336,22 +1297,28 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) { struct cpufreq_policy *policy; unsigned cpu = dev->id; + int ret; dev_dbg(dev, "%s: adding CPU%u\n", __func__, cpu); - if (cpu_online(cpu)) - return cpufreq_online(cpu); + if (cpu_online(cpu)) { + ret = cpufreq_online(cpu); + if (ret) + return ret; + } - /* - * A hotplug notifier will follow and we will handle it as CPU online - * then. For now, just create the sysfs link, unless there is no policy - * or the link is already present. - */ + /* Create sysfs link on CPU registration */ policy = per_cpu(cpufreq_cpu_data, cpu); if (!policy || cpumask_test_and_set_cpu(cpu, policy->real_cpus)) return 0; - return add_cpu_dev_symlink(policy, cpu); + ret = add_cpu_dev_symlink(policy, dev); + if (ret) { + cpumask_clear_cpu(cpu, policy->real_cpus); + cpufreq_offline(cpu); + } + + return ret; } static void cpufreq_offline(unsigned int cpu) @@ -1432,7 +1399,7 @@ static void cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif) cpufreq_offline(cpu); cpumask_clear_cpu(cpu, policy->real_cpus); - remove_cpu_dev_symlink(policy, cpu); + remove_cpu_dev_symlink(policy, dev); if (cpumask_empty(policy->real_cpus)) cpufreq_policy_free(policy, true);