From patchwork Thu Feb 7 07:41:52 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 2109581 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id C4155DFB7B for ; Thu, 7 Feb 2013 07:41:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751535Ab3BGHlz (ORCPT ); Thu, 7 Feb 2013 02:41:55 -0500 Received: from mail-we0-f171.google.com ([74.125.82.171]:65390 "EHLO mail-we0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750900Ab3BGHly (ORCPT ); Thu, 7 Feb 2013 02:41:54 -0500 Received: by mail-we0-f171.google.com with SMTP id u54so1843769wey.30 for ; Wed, 06 Feb 2013 23:41:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=Jrjry0NCDh74ayymLMpKryAsbDjU3//YOwAdkAQaN1I=; b=0hXv40M3Ajs+SGEtteXqMTVwZcAfmO7yj03QNp5CTncm+X2VoY7rVAHHxi1SVDoWBO GmCaHfHqbU/Pb7qsDvw6XB/8KqGxRNt73vPKXanIjrygfxovQSJa9vnE2opCJxTFgxIX uiEMR3Ax/4pQWdJUQLWzZGP7+PS8xcYxn2mOoN/v+VVfyu3WDdNPrXTbzg3dvb3F1jeP sQEjVUnrbfL9ujqyFOGbSK7NvQP+g+PbAc68vzYuXeR9/4ZqxullzUGw+eqrZXBiGBBl Xafdo3cilZWMuwz6Axh6jT3hqw7UFVnnU+BoEvgryWI74YxA9EXYvffyxU5JiLEZn0Yq T30g== MIME-Version: 1.0 X-Received: by 10.194.216.66 with SMTP id oo2mr613266wjc.4.1360222912832; Wed, 06 Feb 2013 23:41:52 -0800 (PST) Received: by 10.194.124.208 with HTTP; Wed, 6 Feb 2013 23:41:52 -0800 (PST) In-Reply-To: <17233164.aBDzy1OViI@vostro.rjw.lan> References: <9180.1360172675@turing-police.cc.vt.edu> <17233164.aBDzy1OViI@vostro.rjw.lan> Date: Thu, 7 Feb 2013 13:11:52 +0530 X-Google-Sender-Auth: I5hQDFpMAa_4A-TiIeXs_BgN9uA Message-ID: Subject: Re: next-20130206 cpufreq - WARN in sysfs_add_one From: Viresh Kumar To: "Rafael J. Wysocki" Cc: Valdis Kletnieks , linux-kernel@vger.kernel.org, cpufreq@vger.kernel.org, linux-pm@vger.kernel.org Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On Thu, Feb 7, 2013 at 2:54 AM, Rafael J. Wysocki wrote: > On Wednesday, February 06, 2013 12:44:35 PM Valdis Kletnieks wrote: >> Seen in dmesg. next-20130128 was OK. Haven't done a bisect, but can >> do so if the offender isn't obvious... > > I suppose this is 73bf0fc "cpufreq: Don't remove sysfs link for policy->cpu". Not really. :) Hi Valdis, First of all i want to confirm something about your system. I am sure it is a multi-policy system (or multi cluster system). i.e. there are more than one clock line for different cpus ? And so multiple struct policy exist simultaneously. Because this crash can only come on those. Anyway, i have tested and pushed a fix here: http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/for-valdis Please test it. For others, the patch is: commit 007dda326f1b1415846671d7fcfbd520f4f16151 Author: Viresh Kumar Date: Thu Feb 7 12:51:27 2013 +0530 cpufreq: governors: Fix WARN_ON() for multi-policy platforms On multi-policy systems there is a single instance of governor for both the policies (if same governor is chosen for both policies). With the code update from following patches: 8eeed09 cpufreq: governors: Get rid of dbs_data->enable field b394058 cpufreq: governors: Reset tunables only for cpufreq_unregister_governor() We are creating/removing sysfs directory of governor for for every call to GOV_START and STOP. This would fail for multi-policy system as there is a per-policy call to START/STOP. This patch reuses the governor->initialized variable to detect total users of governor. Signed-off-by: Viresh Kumar Tested-By: Valdis Kletnieks --- drivers/cpufreq/cpufreq.c | 6 ++++-- drivers/cpufreq/cpufreq_governor.c | 32 +++++++++++++++++++------------- 2 files changed, 23 insertions(+), 15 deletions(-) od_dbs_info->sample_type = OD_NORMAL_SAMPLE; @@ -311,11 +315,13 @@ unlock: mutex_lock(&dbs_data->mutex); mutex_destroy(&cpu_cdbs->timer_mutex); - sysfs_remove_group(cpufreq_global_kobject, - dbs_data->attr_group); - if (dbs_data->governor == GOV_CONSERVATIVE) - cpufreq_unregister_notifier(cs_ops->notifier_block, - CPUFREQ_TRANSITION_NOTIFIER); + if (policy->governor->initialized == 1) { + sysfs_remove_group(cpufreq_global_kobject, + dbs_data->attr_group); + if (dbs_data->governor == GOV_CONSERVATIVE) + cpufreq_unregister_notifier(cs_ops->notifier_block, + CPUFREQ_TRANSITION_NOTIFIER); + } mutex_unlock(&dbs_data->mutex); break; -- 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 ccc598a..3b941a1 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1567,8 +1567,10 @@ static int __cpufreq_governor(struct cpufreq_policy *policy, policy->cpu, event); ret = policy->governor->governor(policy, event); - if (!policy->governor->initialized && (event == CPUFREQ_GOV_START)) - policy->governor->initialized = 1; + if (event == CPUFREQ_GOV_START) + policy->governor->initialized++; + else if (event == CPUFREQ_GOV_STOP) + policy->governor->initialized--; /* we keep one module reference alive for each CPU governed by this CPU */ diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index e4a306c..5a76086 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -247,11 +247,13 @@ int cpufreq_governor_dbs(struct dbs_data *dbs_data, dbs_data->gov_dbs_timer); } - rc = sysfs_create_group(cpufreq_global_kobject, - dbs_data->attr_group); - if (rc) { - mutex_unlock(&dbs_data->mutex); - return rc; + if (!policy->governor->initialized) { + rc = sysfs_create_group(cpufreq_global_kobject, + dbs_data->attr_group); + if (rc) { + mutex_unlock(&dbs_data->mutex); + return rc; + } } /* @@ -262,13 +264,15 @@ int cpufreq_governor_dbs(struct dbs_data *dbs_data, cs_dbs_info->down_skip = 0; cs_dbs_info->enable = 1; cs_dbs_info->requested_freq = policy->cur; - cpufreq_register_notifier(cs_ops->notifier_block, - CPUFREQ_TRANSITION_NOTIFIER); - if (!policy->governor->initialized) + if (!policy->governor->initialized) { + cpufreq_register_notifier(cs_ops->notifier_block, + CPUFREQ_TRANSITION_NOTIFIER); + dbs_data->min_sampling_rate = MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10); + } } else { od_dbs_info->rate_mult = 1;