From patchwork Thu Sep 26 10:29:51 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 20581 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-qa0-f72.google.com (mail-qa0-f72.google.com [209.85.216.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 74B2423A4E for ; Thu, 26 Sep 2013 10:31:52 +0000 (UTC) Received: by mail-qa0-f72.google.com with SMTP id j7sf947847qaq.11 for ; Thu, 26 Sep 2013 03:31:52 -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:in-reply-to:references:in-reply-to:references :x-original-sender:x-original-authentication-results:precedence :mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe; bh=Ccoz1vordO36+lfl5Z1FZjkuBTr77hHpebrZC7+JrUo=; b=HAFrLW/9TKHteB3HkuJ0jElHY35gDAnieX8bBTUf8EUt5ghTysI5f325PMgzN5F0+D K75nYS/v94ybJZwOkn6zk/BYHdGbtH35NrXWBR2/QCijzl9xi/WVRmCKK2ju1d8Eul0X vaC27vmnTmsNa/yAaTH45udCiGBlrQ8RYbVS3/P1vPBPv5tdoimrQ1wvmRdJs829UVHQ 4OVgHvl9cA9a79peOIyjeFV0MgLEzi7yit/MCmg2L7OOt1hJnNGbOZoLGD+48bcto0PT LjAXT+tCTyR8SQQ+rqavU+swHacktGDcjZTKp4GJovt1OlqOqTaX5UQjpDd6dnFo53vI QDhA== X-Gm-Message-State: ALoCoQljxAApjD6CYpdJSZ4T9kHZg2XX0nTtLjM501TrKoWggy3WCtscvNp5coEpP7ob9uImw/DD X-Received: by 10.236.145.34 with SMTP id o22mr46676yhj.22.1380191512331; Thu, 26 Sep 2013 03:31:52 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.49.71.231 with SMTP id y7ls846980qeu.39.gmail; Thu, 26 Sep 2013 03:31:52 -0700 (PDT) X-Received: by 10.52.111.196 with SMTP id ik4mr77151vdb.58.1380191512204; Thu, 26 Sep 2013 03:31:52 -0700 (PDT) Received: from mail-vb0-f45.google.com (mail-vb0-f45.google.com [209.85.212.45]) by mx.google.com with ESMTPS id zw10si212080vdb.96.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 26 Sep 2013 03:31:52 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.212.45 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.212.45; Received: by mail-vb0-f45.google.com with SMTP id e15so669652vbg.18 for ; Thu, 26 Sep 2013 03:31:22 -0700 (PDT) X-Received: by 10.52.231.39 with SMTP id td7mr59728vdc.106.1380191482060; Thu, 26 Sep 2013 03:31:22 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.220.174.196 with SMTP id u4csp357915vcz; Thu, 26 Sep 2013 03:31:21 -0700 (PDT) X-Received: by 10.66.50.131 with SMTP id c3mr4267959pao.111.1380191480951; Thu, 26 Sep 2013 03:31:20 -0700 (PDT) Received: from mail-pa0-f49.google.com (mail-pa0-f49.google.com [209.85.220.49]) by mx.google.com with ESMTPS id rr7si969485pbc.195.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 26 Sep 2013 03:31:20 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.49 is neither permitted nor denied by best guess record for domain of viresh.kumar@linaro.org) client-ip=209.85.220.49; Received: by mail-pa0-f49.google.com with SMTP id ld10so1121406pab.22 for ; Thu, 26 Sep 2013 03:31:20 -0700 (PDT) X-Received: by 10.67.2.4 with SMTP id bk4mr4502597pad.78.1380191480427; Thu, 26 Sep 2013 03:31:20 -0700 (PDT) Received: from localhost ([223.238.78.153]) by mx.google.com with ESMTPSA id k4sm1357504pbd.11.1969.12.31.16.00.00 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 26 Sep 2013 03:31:19 -0700 (PDT) From: Viresh Kumar To: rjw@sisk.pl Cc: linaro-kernel@lists.linaro.org, patches@linaro.org, cpufreq@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, srivatsa.bhat@linux.vnet.ibm.com, nicolas.pitre@linaro.org, Viresh Kumar Subject: [PATCH 1/4] cpufreq: stats: handle cpufreq_unregister_driver() and suspend/resume properly Date: Thu, 26 Sep 2013 15:59:51 +0530 Message-Id: <4e5eacadb26c3a17f8f04fe344bd3cd6ea5a8a48.1380190886.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 1.7.12.rc2.18.g61b472e In-Reply-To: References: In-Reply-To: References: X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: viresh.kumar@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.212.45 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , There are several problems cpufreq stats in the way it handles cpufreq_unregister_driver() and suspend/resume.. - We must not loose data collected so far when suspend/resume happens and so stats directories must not be removed/allocated during these operations, Which is done currently. - cpufreq_stat has registered notifiers with both cpufreq and hotplug. It adds sysfs stats directory with a cpufreq notifier: CPUFREQ_NOTIFY and removes this directory with a notifier from hotplug core. In case cpufreq_unregister_driver() is called (on rmmod cpufreq driver), stats directories per cpu aren't removed as CPUs are still online. The only call cpufreq_stats gets is cpufreq_stats_update_policy_cpu for all CPUs except the last of each policy. And pointer to stat information is stored in the entry for last cpu in per-cpu cpufreq_stats_table. But policy structure would be freed inside cpufreq core and so that will result in memory leak inside cpufreq stats (as we are never freeing memory for stats). Now if we again insert the module cpufreq_register_driver() will be called and we will again allocate stats data and put it on for first cpu of every policy. In case we only have a single cpu per policy we will return with a error from cpufreq_stats_create_table() due to below code: if (per_cpu(cpufreq_stats_table, cpu)) return -EBUSY; And so probably cpufreq stats directory would show up anymore (as it was added inside last policies->kobj which doesn't exist anymore). I haven't tested it though. Also the values in stats files wouldn't be refreshed as we are using the earlier stats structure. - CPUFREQ_NOTIFY is called from cpufreq_set_policy() which is called for scenarios where we don't really want cpufreq_stat_notifier_policy() to get called. For example whenever we are changing anything related to a policy: min/max/current freq, etc.. cpufreq_set_policy() is called and so cpufreq stats is notified. Where we don't do any useful stuff other than simply returning with -EBUSY from cpufreq_stats_create_table(). And so this isn't the right notifier that cpufreq stats.. Due to all above reasons this patch does following changes: - Add new notifiers CPUFREQ_CREATE_POLICY and CPUFREQ_REMOVE_POLICY, which are only called when policy is created/destroyed. They aren't called for suspend/resume paths.. - Use these notifiers in cpufreq_stat_notifier_policy() to create/destory stats sysfs entries. And so cpufreq_unregister_driver() or suspend/resume shouldn't be a problem for cpufreq_stats. - Return early from cpufreq_stat_cpu_callback() for suspend/resume sequence, so that we don't free stats structure. Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 5 +++++ drivers/cpufreq/cpufreq_stats.c | 24 +++++++++++++++++------- include/linux/cpufreq.h | 2 ++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 89b3c52..ab56531 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1086,6 +1086,8 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, ret = cpufreq_add_dev_interface(policy, dev); if (ret) goto err_out_unregister; + blocking_notifier_call_chain(&cpufreq_policy_notifier_list, + CPUFREQ_CREATE_POLICY, policy); } write_lock_irqsave(&cpufreq_driver_lock, flags); @@ -1263,6 +1265,9 @@ static int __cpufreq_remove_dev_finish(struct device *dev, } if (!frozen) { + blocking_notifier_call_chain(&cpufreq_policy_notifier_list, + CPUFREQ_REMOVE_POLICY, policy); + lock_policy_rwsem_read(cpu); kobj = &policy->kobj; cmp = &policy->kobj_unregister; diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c index 4cf0d28..0f71562 100644 --- a/drivers/cpufreq/cpufreq_stats.c +++ b/drivers/cpufreq/cpufreq_stats.c @@ -277,7 +277,7 @@ static void cpufreq_stats_update_policy_cpu(struct cpufreq_policy *policy) static int cpufreq_stat_notifier_policy(struct notifier_block *nb, unsigned long val, void *data) { - int ret; + int ret = 0; struct cpufreq_policy *policy = data; struct cpufreq_frequency_table *table; unsigned int cpu = policy->cpu; @@ -287,15 +287,21 @@ static int cpufreq_stat_notifier_policy(struct notifier_block *nb, return 0; } - if (val != CPUFREQ_NOTIFY) - return 0; table = cpufreq_frequency_get_table(cpu); if (!table) return 0; - ret = cpufreq_stats_create_table(policy, table); - if (ret) - return ret; - return 0; + + if (val == CPUFREQ_CREATE_POLICY) + ret = cpufreq_stats_create_table(policy, table); + else if (val == CPUFREQ_REMOVE_POLICY) { + /* This might already be freed by cpu hotplug notifier */ + if (per_cpu(cpufreq_stats_table, cpu)) { + cpufreq_stats_free_sysfs(cpu); + cpufreq_stats_free_table(cpu); + } + } + + return ret; } static int cpufreq_stat_notifier_trans(struct notifier_block *nb, @@ -340,6 +346,10 @@ static int cpufreq_stat_cpu_callback(struct notifier_block *nfb, { unsigned int cpu = (unsigned long)hcpu; + /* Don't free/allocate stats during suspend/resume */ + if (action & CPU_TASKS_FROZEN) + return 0; + switch (action) { case CPU_DOWN_PREPARE: cpufreq_stats_free_sysfs(cpu); diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index fcabc42..9132fa3 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -259,6 +259,8 @@ static inline void cpufreq_verify_within_limits(struct cpufreq_policy *policy, #define CPUFREQ_NOTIFY (2) #define CPUFREQ_START (3) #define CPUFREQ_UPDATE_POLICY_CPU (4) +#define CPUFREQ_CREATE_POLICY (5) +#define CPUFREQ_REMOVE_POLICY (6) #ifdef CONFIG_CPU_FREQ int cpufreq_register_notifier(struct notifier_block *nb, unsigned int list);