From: Viresh Kumar <viresh.kumar@linaro.org>
To: Jacob Shin <jacob.shin@amd.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>,
cpufreq@vger.kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, linaro-kernel@lists.linaro.org,
robin.randhawa@arm.com, Steve.Bannister@arm.com,
Liviu.Dudau@arm.com, charles.garcia-tobin@arm.com,
Arvind Chauhan <Arvind.Chauhan@arm.com>
Subject: Re: [PATCH V3 2/4] cpufreq: governor: Implement per policy instances of governors
Date: Wed, 27 Mar 2013 01:02:15 +0530 [thread overview]
Message-ID: <CAKohpons4Tr8F2RuRGCxbOFmJaK9YbAafV-3v6=cijRupe2hUA@mail.gmail.com> (raw)
In-Reply-To: <20130326152059.GA1698@jshin-Toonie>
[-- Attachment #1: Type: text/plain, Size: 6072 bytes --]
On 26 March 2013 20:50, Jacob Shin <jacob.shin@amd.com> wrote:
> Hi, latest bleeding-edge is spewing this out on boot:
>
> [ 3.585157] ------------[ cut here ]------------
> [ 3.592227] WARNING: at fs/sysfs/dir.c:536 sysfs_add_one+0xc8/0x100()
> [ 3.599521] Hardware name: Dinar
> [ 3.606878] sysfs: cannot create duplicate filename '/devices/system/cpu/cpufreq/ondemand'
> [ 3.614634] Modules linked in:
> [ 3.622382] Pid: 1, comm: swapper/0 Not tainted 3.9.0-rc4+ #7
> [ 3.630305] Call Trace:
> [ 3.638251] [<ffffffff810589cf>] warn_slowpath_common+0x7f/0xc0
> [ 3.646435] [<ffffffff81058ac6>] warn_slowpath_fmt+0x46/0x50
> [ 3.654586] [<ffffffff8133e2f0>] ? strlcat+0x60/0x80
> [ 3.662765] [<ffffffff811fe7d8>] sysfs_add_one+0xc8/0x100
> [ 3.670977] [<ffffffff811fe9cc>] create_dir+0x7c/0xd0
> [ 3.679239] [<ffffffff811fecaf>] sysfs_create_subdir+0x1f/0x30
> [ 3.687601] [<ffffffff812006c4>] internal_create_group+0x64/0x210
> [ 3.696098] [<ffffffff812008a3>] sysfs_create_group+0x13/0x20
> [ 3.704700] [<ffffffff816bf800>] cpufreq_governor_dbs+0x400/0x590
> [ 3.713401] [<ffffffff816bdc37>] od_cpufreq_governor_dbs+0x17/0x20
> [ 3.722191] [<ffffffff816bb437>] __cpufreq_governor+0x47/0xc0
> [ 3.731071] [<ffffffff816bb94d>] __cpufreq_set_policy+0x19d/0x1b0
> [ 3.739968] [<ffffffff816bca89>] cpufreq_add_dev_interface+0x259/0x2b0
> [ 3.748960] [<ffffffff813cdce6>] ? acpi_processor_get_performance_info+0x21c/0x452
> [ 3.758099] [<ffffffff816bc210>] ? cpufreq_update_policy+0x130/0x130
> [ 3.767366] [<ffffffff816bce90>] cpufreq_add_dev+0x3b0/0x4d0
> [ 3.776659] [<ffffffff821579d4>] ? cpufreq_gov_dbs_init+0x12/0x12
> [ 3.785985] [<ffffffff814e6a39>] subsys_interface_register+0x89/0xd0
> [ 3.795452] [<ffffffff816baf5e>] cpufreq_register_driver+0x8e/0x180
> [ 3.804919] [<ffffffff82157aca>] acpi_cpufreq_init+0xf6/0x1f8
> [ 3.814360] [<ffffffff814f5030>] ? set_trace_device+0x80/0x80
> [ 3.823558] [<ffffffff8100206f>] do_one_initcall+0x3f/0x170
> [ 3.832476] [<ffffffff8211b00a>] kernel_init_freeable+0x13e/0x1cd
> [ 3.841131] [<ffffffff8211a88e>] ? do_early_param+0x86/0x86
> [ 3.849506] [<ffffffff817f4c20>] ? rest_init+0x80/0x80
> [ 3.857557] [<ffffffff817f4c2e>] kernel_init+0xe/0xf0
> [ 3.865260] [<ffffffff8181edec>] ret_from_fork+0x7c/0xb0
> [ 3.872886] [<ffffffff817f4c20>] ? rest_init+0x80/0x80
> [ 3.880456] ---[ end trace 1a5c6247c6d9b0ac ]---
> [ 3.888201] ------------[ cut here ]------------
>
> This warning is repeated for number of cpus - 1 times.
>
> And when I do:
>
> $ cat /sys/devices/system/cpu/cpufreq/ondemand/up_threshold
>
> [ 489.103388] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
> [ 489.112064] IP: [<ffffffff816be02c>] show_up_threshold+0x1c/0x30
> [ 489.120511] PGD a285e6067 PUD a27085067 PMD 0
> [ 489.128690] Oops: 0000 [#1] SMP
> [ 489.136521] Modules linked in:
> [ 489.144134] CPU 15
> [ 489.144229] Pid: 1565, comm: cat Tainted: G W 3.9.0-rc4+ #7 AMD Dinar/Dinar
> [ 489.159654] RIP: 0010:[<ffffffff816be02c>] [<ffffffff816be02c>] show_up_threshold+0x1c/0x30
> [ 489.167864] RSP: 0018:ffff880423859e88 EFLAGS: 00010246
> [ 489.176043] RAX: 0000000000000000 RBX: ffff880a271188c0 RCX: ffffffff81a41810
> [ 489.184372] RDX: 0000000000000000 RSI: ffffffff81e02dae RDI: ffffffff820d7860
> [ 489.184373] RBP: ffff880423859e88 R08: ffffea0028b6df80 R09: 00000000001f05b8
> [ 489.184374] R10: 0000000000001b97 R11: 0000000000000246 R12: ffff880423859f50
> [ 489.184374] R13: 0000000000008000 R14: ffff880a271188a0 R15: ffff8804251aa070
> [ 489.184377] FS: 00007f3278b31700(0000) GS:ffff880a2fcc0000(0000) knlGS:0000000000000000
> [ 489.184378] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 489.184378] CR2: 0000000000000010 CR3: 0000000a2db82000 CR4: 00000000000407e0
> [ 489.184380] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 489.184381] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 489.184382] Process cat (pid: 1565, threadinfo ffff880423858000, task ffff880424805c00)
> [ 489.184382] Stack:
> [ 489.184389] ffff880423859e98 ffffffff8133814f ffff880423859ef8 ffffffff811fd62a
> [ 489.184394] 000000002bf67be2 ffff8808260b7a80 ffffffff81a41810 00000000012c8000
> [ 489.184399] ffff880423859ef8 0000000000008000 00000000012c8000 ffff880423859f50
> [ 489.184400] Call Trace:
> [ 489.184406] [<ffffffff8133814f>] kobj_attr_show+0xf/0x30
> [ 489.184411] [<ffffffff811fd62a>] sysfs_read_file+0xaa/0x190
> [ 489.184415] [<ffffffff81187e30>] vfs_read+0xb0/0x180
> [ 489.184418] [<ffffffff81187f52>] sys_read+0x52/0xa0
> [ 489.184422] [<ffffffff8181a7fe>] ? do_page_fault+0xe/0x10
> [ 489.184426] [<ffffffff8181ee99>] system_call_fastpath+0x16/0x1b
> [ 489.184441] Code: 52 08 e8 78 2c c8 ff 5d 48 98 c3 0f 1f 40 00 66 66 66 66 90 55 48 8b 57 70 48 89 f0 48 89 c7 48 c7 c6 ae 2d e0 81 31 c0 48 89 e5 <48> 8b 52 10 8b 52 0c e8 48 2c c8 ff 5d 48 98 c3 0f 1f 40 00 66
> [ 489.184443] RIP [<ffffffff816be02c>] show_up_threshold+0x1c/0x30
> [ 489.184443] RSP <ffff880423859e88>
> [ 489.184444] CR2: 0000000000000010
> [ 489.184507] ---[ end trace 1a5c6247c6d9b0c3 ]---
>
> Any ideas?
Yes, i believe i have enough idea about it :)
There are two kind of systems i know:
1 - Single group of cpus controlled by a single clock line,
i.e. only one policy instance at any time
2 - multipolicy systems where we have more than one group of cpus
and every group have one clock line.
For the second case also there are two cases:
2.1 - support have_multiple_policies (i.e. have separate instance of governor
for each policy struct)
2.2 - doesn't support have_multiple_policies
The last one (2.2) is broken with my patch and attached is the fix. I
have tested
it on my Lenovo Thinkpad which is more like 2.2 case.
cat of cpufreq/ondemand/** is still broken and i am too tired of
fixing it now...
Its already midnight here 01:01 AM.
--
viresh
[-- Attachment #2: 0001-fixup-cpufreq-governor-Implement-per-policy-instance.patch --]
[-- Type: application/octet-stream, Size: 7337 bytes --]
From c95ca544325f40f55858e02526d0bd6a1740480d Mon Sep 17 00:00:00 2001
Message-Id: <c95ca544325f40f55858e02526d0bd6a1740480d.1364322587.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Tue, 26 Mar 2013 23:20:18 +0530
Subject: [PATCH] fixup! cpufreq: governor: Implement per policy instances of
governors
---
drivers/cpufreq/cpufreq.c | 13 +++++++
drivers/cpufreq/cpufreq_governor.c | 74 +++++++++++++++++++++++++++-----------
include/linux/cpufreq.h | 2 ++
3 files changed, 68 insertions(+), 21 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 8f2a603..3c79025 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -136,6 +136,11 @@ struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy)
return cpufreq_global_kobject;
}
+bool have_multiple_policies(void)
+{
+ return cpufreq_driver->have_multiple_policies;
+}
+
static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, bool sysfs)
{
struct cpufreq_policy *data;
@@ -1561,6 +1566,13 @@ static int __cpufreq_governor(struct cpufreq_policy *policy,
policy->cpu, event);
ret = policy->governor->governor(policy, event);
+ if (!ret) {
+ if (event == CPUFREQ_GOV_POLICY_INIT)
+ policy->governor->initialized++;
+ else if (event == CPUFREQ_GOV_POLICY_EXIT)
+ policy->governor->initialized--;
+ }
+
/* we keep one module reference alive for
each CPU governed by this CPU */
if ((event != CPUFREQ_GOV_START) || ret)
@@ -1584,6 +1596,7 @@ int cpufreq_register_governor(struct cpufreq_governor *governor)
mutex_lock(&cpufreq_governor_mutex);
+ governor->initialized = 0;
err = -EBUSY;
if (__find_governor(governor->name) == NULL) {
err = 0;
diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
index 41e5e56..f29feb4 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -29,6 +29,9 @@
#include "cpufreq_governor.h"
+/* Common data for platforms that don't need governor instance per policy */
+struct dbs_data *gdbs_data;
+
static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
{
u64 idle_time;
@@ -216,10 +219,9 @@ static void set_sampling_rate(struct dbs_data *dbs_data,
int cpufreq_governor_dbs(struct cpufreq_policy *policy,
struct common_dbs_data *cdata, unsigned int event)
{
- struct dbs_data *dbs_data = policy->governor_data;
+ struct dbs_data *dbs_data;
struct od_cpu_dbs_info_s *od_dbs_info = NULL;
struct cs_cpu_dbs_info_s *cs_dbs_info = NULL;
- struct cs_ops *cs_ops = NULL;
struct od_ops *od_ops = NULL;
struct od_dbs_tuners *od_tuners = NULL;
struct cs_dbs_tuners *cs_tuners = NULL;
@@ -228,11 +230,22 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
int io_busy = 0;
int rc;
+ if (have_multiple_policies())
+ dbs_data = policy->governor_data;
+ else
+ dbs_data = gdbs_data;
+
WARN_ON(!dbs_data && (event != CPUFREQ_GOV_POLICY_INIT));
switch (event) {
case CPUFREQ_GOV_POLICY_INIT:
- WARN_ON(dbs_data);
+ if (have_multiple_policies()) {
+ WARN_ON(dbs_data);
+ } else if (dbs_data) {
+ policy->governor_data = dbs_data;
+ return 0;
+ }
+
dbs_data = kzalloc(sizeof(*dbs_data), GFP_KERNEL);
if (!dbs_data) {
pr_err("%s: POLICY_INIT: kzalloc failed\n", __func__);
@@ -246,6 +259,15 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
kfree(dbs_data);
return rc;
}
+
+ rc = sysfs_create_group(get_governor_parent_kobj(policy),
+ dbs_data->cdata->attr_group);
+ if (rc) {
+ cdata->exit(dbs_data);
+ kfree(dbs_data);
+ return rc;
+ }
+
policy->governor_data = dbs_data;
/* policy latency is in nS. Convert it to uS first */
@@ -258,10 +280,36 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
MIN_LATENCY_MULTIPLIER * latency);
set_sampling_rate(dbs_data, max(dbs_data->min_sampling_rate,
latency * LATENCY_MULTIPLIER));
+
+ if (dbs_data->cdata->governor == GOV_CONSERVATIVE) {
+ struct cs_ops *cs_ops = dbs_data->cdata->gov_ops;
+
+ cpufreq_register_notifier(cs_ops->notifier_block,
+ CPUFREQ_TRANSITION_NOTIFIER);
+ }
+
+ if (!have_multiple_policies())
+ gdbs_data = dbs_data;
+
return 0;
case CPUFREQ_GOV_POLICY_EXIT:
- cdata->exit(dbs_data);
- kfree(dbs_data);
+ if ((policy->governor->initialized == 1) ||
+ have_multiple_policies()) {
+ sysfs_remove_group(get_governor_parent_kobj(policy),
+ dbs_data->cdata->attr_group);
+
+ if (dbs_data->cdata->governor == GOV_CONSERVATIVE) {
+ struct cs_ops *cs_ops = dbs_data->cdata->gov_ops;
+
+ cpufreq_register_notifier(cs_ops->notifier_block,
+ CPUFREQ_TRANSITION_NOTIFIER);
+ }
+
+ cdata->exit(dbs_data);
+ kfree(dbs_data);
+ gdbs_data = NULL;
+ }
+
policy->governor_data = NULL;
return 0;
}
@@ -273,7 +321,6 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
cs_dbs_info = dbs_data->cdata->get_cpu_dbs_info_s(cpu);
sampling_rate = cs_tuners->sampling_rate;
ignore_nice = cs_tuners->ignore_nice;
- cs_ops = dbs_data->cdata->gov_ops;
} else {
od_tuners = dbs_data->tuners;
od_dbs_info = dbs_data->cdata->get_cpu_dbs_info_s(cpu);
@@ -307,13 +354,6 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
dbs_data->cdata->gov_dbs_timer);
}
- rc = sysfs_create_group(get_governor_parent_kobj(policy),
- dbs_data->cdata->attr_group);
- if (rc) {
- mutex_unlock(&dbs_data->mutex);
- return rc;
- }
-
/*
* conservative does not implement micro like ondemand
* governor, thus we are bound to jiffes/HZ
@@ -322,9 +362,6 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
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);
} else {
od_dbs_info->rate_mult = 1;
od_dbs_info->sample_type = OD_NORMAL_SAMPLE;
@@ -349,11 +386,6 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
mutex_lock(&dbs_data->mutex);
mutex_destroy(&cpu_cdbs->timer_mutex);
- sysfs_remove_group(get_governor_parent_kobj(policy),
- dbs_data->cdata->attr_group);
- if (dbs_data->cdata->governor == GOV_CONSERVATIVE)
- cpufreq_unregister_notifier(cs_ops->notifier_block,
- CPUFREQ_TRANSITION_NOTIFIER);
mutex_unlock(&dbs_data->mutex);
break;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 8fe9b10..f253a3e 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -187,6 +187,7 @@ static inline unsigned long cpufreq_scale(unsigned long old, u_int div, u_int mu
struct cpufreq_governor {
char name[CPUFREQ_NAME_LEN];
+ int initialized;
int (*governor) (struct cpufreq_policy *policy,
unsigned int event);
ssize_t (*show_setspeed) (struct cpufreq_policy *policy,
@@ -323,6 +324,7 @@ const char *cpufreq_get_current_driver(void);
int cpufreq_get_policy(struct cpufreq_policy *policy, unsigned int cpu);
int cpufreq_update_policy(unsigned int cpu);
struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy);
+bool have_multiple_policies(void);
#ifdef CONFIG_CPU_FREQ
/* query the current CPU frequency (in kHz). If zero, cpufreq couldn't detect it */
--
1.7.12.rc2.18.g61b472e
next prev parent reply other threads:[~2013-03-26 19:32 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-04 7:37 [PATCH V3 0/4] CPUFreq: Implement per policy instances of governors Viresh Kumar
2013-03-04 7:37 ` [PATCH V3 1/4] cpufreq: Add per policy governor-init/exit infrastructure Viresh Kumar
2013-03-04 7:37 ` [PATCH V3 2/4] cpufreq: governor: Implement per policy instances of governors Viresh Kumar
2013-03-20 5:29 ` Viresh Kumar
2013-03-21 23:44 ` Rafael J. Wysocki
2013-03-22 2:20 ` Viresh Kumar
2013-03-22 11:55 ` Rafael J. Wysocki
2013-03-22 11:51 ` Viresh Kumar
2013-03-22 12:11 ` Rafael J. Wysocki
2013-03-22 12:05 ` Viresh Kumar
2013-03-26 15:20 ` Jacob Shin
2013-03-26 19:32 ` Viresh Kumar [this message]
2013-03-26 19:48 ` Jacob Shin
2013-03-27 4:29 ` Viresh Kumar
2013-03-27 10:04 ` Viresh Kumar
2013-03-27 11:35 ` Viresh Kumar
2013-03-27 14:37 ` Viresh Kumar
2013-03-04 7:37 ` [PATCH V3 3/4] cpufreq: Get rid of "struct global_attr" Viresh Kumar
2013-03-04 7:37 ` [PATCH V3 4/4] cpufreq: Add Kconfig option to enable/disable have_multiple_policies Viresh Kumar
2013-03-11 23:38 ` Rafael J. Wysocki
2013-03-12 0:55 ` Viresh Kumar
2013-03-13 21:41 ` Rafael J. Wysocki
2013-03-14 3:09 ` Viresh Kumar
2013-03-20 0:20 ` Rafael J. Wysocki
2013-03-20 4:23 ` Viresh Kumar
2013-03-20 5:16 ` Viresh Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKohpons4Tr8F2RuRGCxbOFmJaK9YbAafV-3v6=cijRupe2hUA@mail.gmail.com' \
--to=viresh.kumar@linaro.org \
--cc=Arvind.Chauhan@arm.com \
--cc=Liviu.Dudau@arm.com \
--cc=Steve.Bannister@arm.com \
--cc=charles.garcia-tobin@arm.com \
--cc=cpufreq@vger.kernel.org \
--cc=jacob.shin@amd.com \
--cc=linaro-kernel@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@sisk.pl \
--cc=robin.randhawa@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).