From patchwork Thu Jul 10 12:41:44 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 33419 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-pa0-f71.google.com (mail-pa0-f71.google.com [209.85.220.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 7552D203C0 for ; Thu, 10 Jul 2014 12:42:07 +0000 (UTC) Received: by mail-pa0-f71.google.com with SMTP id eu11sf59390596pac.2 for ; Thu, 10 Jul 2014 05:42:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:cc:subject :date:message-id:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=7bSFvkNWczFVNHin79tYQIQItZxJTDqhEtZ85kXwyeM=; b=VLikAKlSPv7KGWTBUbac7zp9nT/t3b3/ZrgcZ8oY7fbOfubN0GyvwzNlVbBvN2Y+jv SCdCCzxP0CPT5A9aK8lZFhnwXuxcxkAQMsVQoTrK4k2cESnjC19WNO4VAunIdhBZBSL3 Lb2J/1f0dPIj8noqv7BRfnYwXQjaqpyB/YLQc8WLUbukw/6MOnyeIjOVaBj5swpS7t4A j2iWRUFbnkzJl8H9g2LpGZHFj6g68XeqKqbLMhouEAWo8/ATgYjU+FWz7P93ELaduRv9 dk+MGmak9pBw763uhFDN01Umhm7JNrzmu8uqM5HXQG/3l3dtIUyyDG+DpdO8aUmaSeaZ MRhQ== X-Gm-Message-State: ALoCoQn+AJ3fgIdxyRF/iKxodkG/3cVj7JqBizYINh4m4jhtG4Nb2yfGJWZqbYONz2gR4uGMDAdm X-Received: by 10.66.162.38 with SMTP id xx6mr5368828pab.46.1404996127181; Thu, 10 Jul 2014 05:42:07 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.85.213 with SMTP id n79ls51041qgd.29.gmail; Thu, 10 Jul 2014 05:42:07 -0700 (PDT) X-Received: by 10.220.160.67 with SMTP id m3mr128692vcx.56.1404996127073; Thu, 10 Jul 2014 05:42:07 -0700 (PDT) Received: from mail-vc0-f174.google.com (mail-vc0-f174.google.com [209.85.220.174]) by mx.google.com with ESMTPS id k14si21092697vdg.49.2014.07.10.05.42.07 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 10 Jul 2014 05:42:07 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.174 as permitted sender) client-ip=209.85.220.174; Received: by mail-vc0-f174.google.com with SMTP id hy4so10299347vcb.5 for ; Thu, 10 Jul 2014 05:42:07 -0700 (PDT) X-Received: by 10.52.252.4 with SMTP id zo4mr38087276vdc.20.1404996126941; Thu, 10 Jul 2014 05:42:06 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp129599vcb; Thu, 10 Jul 2014 05:42:06 -0700 (PDT) X-Received: by 10.68.250.3 with SMTP id yy3mr39523455pbc.56.1404996126018; Thu, 10 Jul 2014 05:42:06 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id nu16si8543360pdb.45.2014.07.10.05.42.05 for ; Thu, 10 Jul 2014 05:42:05 -0700 (PDT) Received-SPF: none (google.com: linux-pm-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753497AbaGJMl6 (ORCPT + 14 others); Thu, 10 Jul 2014 08:41:58 -0400 Received: from mail-pa0-f54.google.com ([209.85.220.54]:37259 "EHLO mail-pa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753371AbaGJMl5 (ORCPT ); Thu, 10 Jul 2014 08:41:57 -0400 Received: by mail-pa0-f54.google.com with SMTP id et14so11113737pad.13 for ; Thu, 10 Jul 2014 05:41:56 -0700 (PDT) X-Received: by 10.70.102.66 with SMTP id fm2mr16851161pdb.102.1404996116512; Thu, 10 Jul 2014 05:41:56 -0700 (PDT) Received: from localhost ([122.167.123.210]) by mx.google.com with ESMTPSA id se3sm61849093pbb.80.2014.07.10.05.41.52 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 10 Jul 2014 05:41:55 -0700 (PDT) From: Viresh Kumar To: rjw@rjwysocki.net Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, arvind.chauhan@arm.com, srivatsa@mit.edu, skannan@codeaurora.org, ybu@qti.qualcomm.com, Viresh Kumar , Stable Subject: [PATCH V2] cpufreq: move policy kobj to policy->cpu at resume Date: Thu, 10 Jul 2014 18:11:44 +0530 Message-Id: <1eff59d9b51f8ade67bc1cf9f10683f9463ec9f6.1404996041.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.0.0.rc2 Sender: linux-pm-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: viresh.kumar@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.174 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , This is only relevant to implementations with multiple clusters, where clusters have separate clock lines but all CPUs within a cluster share it. Consider a dual cluster platform with 2 cores per cluster. During suspend we start offlining CPUs from 1 to 3. When CPU2 is remove, policy->kobj would be moved to CPU3 and when CPU3 goes down we wouldn't free policy or its kobj. Now on resume, we will get CPU2 before CPU3 and will call __cpufreq_add_dev(). We will recover the old policy and update policy->cpu from 3 to 2 from update_policy_cpu(). But the kobj is still tied to CPU3 and wasn't moved to CPU2. We wouldn't create a link for CPU2, but would try that while bringing CPU3 online. Which will report errors as CPU3 already has kobj assigned to it. This bug got introduced with commit 42f921a, which overlooked this scenario. To fix this, lets move kobj to the new policy->cpu while bringing first CPU of a cluster back. We already have update_policy_cpu() routine which can be updated with this change. That would get rid of the cpufreq_nominate_new_policy_cpu() as update_policy_cpu() is also called on CPU removal. To achieve that we add another parameter to update_policy_cpu() as cpu_dev is present with both the callers. Fixes: ("42f921a cpufreq: remove sysfs files for CPUs which failed to come back after resume") Cc: Stable # 3.13+ Reported-by: Bu Yitian Reported-by: Saravana Kannan Signed-off-by: Viresh Kumar --- V1->V2: Move kobject_move() call to update_policy_cpu(), which makes cpufreq_nominate_new_policy_cpu() almost empty. So we remove it completely. @Yitian: Sorry, but you need to test this again as there were enough modifications in V2. drivers/cpufreq/cpufreq.c | 73 +++++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 62259d2..c81d9ec6 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1076,10 +1076,20 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy) kfree(policy); } -static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu) +static int update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu, + struct device *cpu_dev) { + int ret; + if (WARN_ON(cpu == policy->cpu)) - return; + return 0; + + /* Move kobject to the new policy->cpu */ + ret = kobject_move(&policy->kobj, &cpu_dev->kobj); + if (ret) { + pr_err("%s: Failed to move kobj: %d\n", __func__, ret); + return ret; + } down_write(&policy->rwsem); @@ -1090,6 +1100,8 @@ static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu) blocking_notifier_call_chain(&cpufreq_policy_notifier_list, CPUFREQ_UPDATE_POLICY_CPU, policy); + + return 0; } static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) @@ -1154,7 +1166,7 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) * by invoking update_policy_cpu(). */ if (recover_policy && cpu != policy->cpu) - update_policy_cpu(policy, cpu); + WARN_ON(update_policy_cpu(policy, cpu, dev)); else policy->cpu = cpu; @@ -1307,38 +1319,11 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) return __cpufreq_add_dev(dev, sif); } -static int cpufreq_nominate_new_policy_cpu(struct cpufreq_policy *policy, - unsigned int old_cpu) -{ - struct device *cpu_dev; - int ret; - - /* first sibling now owns the new sysfs dir */ - cpu_dev = get_cpu_device(cpumask_any_but(policy->cpus, old_cpu)); - - sysfs_remove_link(&cpu_dev->kobj, "cpufreq"); - ret = kobject_move(&policy->kobj, &cpu_dev->kobj); - if (ret) { - pr_err("%s: Failed to move kobj: %d\n", __func__, ret); - - down_write(&policy->rwsem); - cpumask_set_cpu(old_cpu, policy->cpus); - up_write(&policy->rwsem); - - ret = sysfs_create_link(&cpu_dev->kobj, &policy->kobj, - "cpufreq"); - - return -EINVAL; - } - - return cpu_dev->id; -} - static int __cpufreq_remove_dev_prepare(struct device *dev, struct subsys_interface *sif) { unsigned int cpu = dev->id, cpus; - int new_cpu, ret; + int ret; unsigned long flags; struct cpufreq_policy *policy; @@ -1378,14 +1363,28 @@ static int __cpufreq_remove_dev_prepare(struct device *dev, if (cpu != policy->cpu) { sysfs_remove_link(&dev->kobj, "cpufreq"); } else if (cpus > 1) { - new_cpu = cpufreq_nominate_new_policy_cpu(policy, cpu); - if (new_cpu >= 0) { - update_policy_cpu(policy, new_cpu); + /* Nominate new CPU */ + int new_cpu = cpumask_any_but(policy->cpus, cpu); + struct device *cpu_dev = get_cpu_device(new_cpu); - if (!cpufreq_suspended) - pr_debug("%s: policy Kobject moved to cpu: %d from: %d\n", - __func__, new_cpu, cpu); + sysfs_remove_link(&cpu_dev->kobj, "cpufreq"); + ret = update_policy_cpu(policy, new_cpu, cpu_dev); + if (ret) { + /* + * To supress compilation warning about return value of + * sysfs_create_link(). + */ + int temp; + + /* Create link again if we failed. */ + temp = sysfs_create_link(&cpu_dev->kobj, &policy->kobj, + "cpufreq"); + return ret; } + + if (!cpufreq_suspended) + pr_debug("%s: policy Kobject moved to cpu: %d from: %d\n", + __func__, new_cpu, cpu); } else if (cpufreq_driver->stop_cpu && cpufreq_driver->setpolicy) { cpufreq_driver->stop_cpu(policy); }