| Message ID | CAKohponTZ29BX6CrUvFMy9RXV==0wRFu4C-W1xd6t1RmLASTvQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wednesday, March 20, 2013 10:59:13 AM Viresh Kumar wrote: > On 4 March 2013 13:07, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > Currently, there can't be multiple instances of single governor_type. If we have > > a multi-package system, where we have multiple instances of struct policy (per > > package), we can't have multiple instances of same governor. i.e. We can't have > > multiple instances of ondemand governor for multiple packages. > > > > Governors directory in sysfs is created at /sys/devices/system/cpu/cpufreq/ > > governor-name/. Which again reflects that there can be only one instance of a > > governor_type in the system. > > > > This is a bottleneck for multicluster system, where we want different packages > > to use same governor type, but with different tunables. > > > > This patch uses the infrastructure provided by earlier patch and implements > > init/exit routines for ondemand and conservative governors. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > As discussed in other patch thread, i dropped "cpufreq: Add Kconfig option > to enable/disable have_multiple_policies" patch and following is the fixup > to this patch: > > I have queued all patches i had for 3.10 here: > > http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/for-3.10 OK, applied these to linux-pm.git/bleeding-edge. At the moment bleeding-edge and linux-next diverged slightly on cpufreq, but I hope the bleeding-edge material won't cause build problems to occur, so I'll be able to move it to linux-next shortly. > commit f02fca9a2478088c4f7dadf82d998ae007a56285 > Author: Viresh Kumar <viresh.kumar@linaro.org> > Date: Wed Mar 20 10:50:33 2013 +0530 > > fixup! cpufreq: governor: Implement per policy instances of governors I'd actually prefer you to post complete updated patches instead of these fixups. They are real PITA for me and probably for everybody else trying to follow the cpufreq development recently. Thanks, Rafael > --- > drivers/cpufreq/cpufreq.c | 8 ++++++++ > include/linux/cpufreq.h | 22 ++++++++-------------- > 2 files changed, 16 insertions(+), 14 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index a843855..3d83b02 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -128,6 +128,14 @@ void disable_cpufreq(void) > static LIST_HEAD(cpufreq_governor_list); > static DEFINE_MUTEX(cpufreq_governor_mutex); > > +struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy) > +{ > + if (cpufreq_driver->have_multiple_policies) > + return &policy->kobj; > + else > + return cpufreq_global_kobject; > +} > + > static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, bool sysfs) > { > struct cpufreq_policy *data; > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index 6e1abd2..805c4d3 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -107,11 +107,6 @@ struct cpufreq_policy { > unsigned int policy; /* see above */ > struct cpufreq_governor *governor; /* see below */ > void *governor_data; > - /* This should be set by init() of platforms having multiple > - * clock-domains, i.e. supporting multiple policies. With this sysfs > - * directories of governor would be created in cpu/cpu<num>/cpufreq/ > - * directory */ > - bool have_multiple_policies; > > struct work_struct update; /* if update_policy() needs to be > * called, but you're in IRQ context */ > @@ -139,15 +134,6 @@ static inline bool policy_is_shared(struct > cpufreq_policy *policy) > return cpumask_weight(policy->cpus) > 1; > } > > -static inline struct kobject * > -get_governor_parent_kobj(struct cpufreq_policy *policy) > -{ > - if (policy->have_multiple_policies) > - return &policy->kobj; > - else > - return cpufreq_global_kobject; > -} > - > /******************** cpufreq transition notifiers *******************/ > > #define CPUFREQ_PRECHANGE (0) > @@ -245,6 +231,13 @@ struct cpufreq_driver { > struct module *owner; > char name[CPUFREQ_NAME_LEN]; > u8 flags; > + /* > + * This should be set by init() of platforms having multiple > + * clock-domains, i.e. supporting multiple policies. With this sysfs > + * directories of governor would be created in cpu/cpu<num>/cpufreq/ > + * directory > + */ > + bool have_multiple_policies; > > /* needed by all drivers */ > int (*init) (struct cpufreq_policy *policy); > @@ -329,6 +322,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); > > #ifdef CONFIG_CPU_FREQ > /* query the current CPU frequency (in kHz). If zero, cpufreq > couldn't detect it */ > -- > To unsubscribe from this list: send the line "unsubscribe cpufreq" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22 March 2013 05:14, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Wednesday, March 20, 2013 10:59:13 AM Viresh Kumar wrote: >> I have queued all patches i had for 3.10 here: >> >> http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/for-3.10 > > OK, applied these to linux-pm.git/bleeding-edge. Thanks. > At the moment bleeding-edge and linux-next diverged slightly on cpufreq, but > I hope the bleeding-edge material won't cause build problems to occur, so I'll > be able to move it to linux-next shortly. There shouldn't be any build problems not because i have done all build testing properly BUT because my tree is under continuously surveillance by Fengguang's bot. And any problem with my branches is reported very early :) >> commit f02fca9a2478088c4f7dadf82d998ae007a56285 >> Author: Viresh Kumar <viresh.kumar@linaro.org> >> Date: Wed Mar 20 10:50:33 2013 +0530 >> >> fixup! cpufreq: governor: Implement per policy instances of governors > > I'd actually prefer you to post complete updated patches instead of these > fixups. They are real PITA for me and probably for everybody else trying > to follow the cpufreq development recently. Hmm... I always thought fixups are way easy to review (and i still believe that's true) as they just contain what got changed and so people don't have to review whole patch again. BUT people who are looking for complete patches to apply would be annoyed by this and hence i always show them path of my repo where they can find it. So, what i may do is, post fixups and then resend patches. So that reviewer knows what changed and others can have complete patches too. -- viresh -- 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
On Fri, Mar 22, 2013 at 5:25 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Friday, March 22, 2013 07:50:54 AM Viresh Kumar wrote: >> Hmm... I always thought fixups are way easy to review (and i still >> believe that's >> true) as they just contain what got changed and so people don't have to review >> whole patch again. > > They won't have to if you write in the preamble what the differences from > previous versions are. You didn't get me.. How will the reviewer check if author has done what he is saying in preamble and he hasn't broken anything new? >> BUT people who are looking for complete patches to apply >> would be annoyed by this and hence i always show them path of my repo >> where they can find it. > > The problem with this approach is that the complete patches never make it to > the mailing lists and people have problems with connecting commits to > previously posted patches. > > Moreover, it is *much* more convenient to me to take patches from kernel.org > patchwork than from your repos, with all due respect. I understand.. I push them to repo only when they are broken, otherwise not. -- 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
On Friday, March 22, 2013 07:50:54 AM Viresh Kumar wrote: > On 22 March 2013 05:14, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Wednesday, March 20, 2013 10:59:13 AM Viresh Kumar wrote: > > >> I have queued all patches i had for 3.10 here: > >> > >> http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/for-3.10 > > > > OK, applied these to linux-pm.git/bleeding-edge. > > Thanks. > > > At the moment bleeding-edge and linux-next diverged slightly on cpufreq, but > > I hope the bleeding-edge material won't cause build problems to occur, so I'll > > be able to move it to linux-next shortly. > > There shouldn't be any build problems not because i have done all build testing > properly BUT because my tree is under continuously surveillance by Fengguang's > bot. And any problem with my branches is reported very early :) > > >> commit f02fca9a2478088c4f7dadf82d998ae007a56285 > >> Author: Viresh Kumar <viresh.kumar@linaro.org> > >> Date: Wed Mar 20 10:50:33 2013 +0530 > >> > >> fixup! cpufreq: governor: Implement per policy instances of governors > > > > I'd actually prefer you to post complete updated patches instead of these > > fixups. They are real PITA for me and probably for everybody else trying > > to follow the cpufreq development recently. > > Hmm... I always thought fixups are way easy to review (and i still > believe that's > true) as they just contain what got changed and so people don't have to review > whole patch again. They won't have to if you write in the preamble what the differences from previous versions are. > BUT people who are looking for complete patches to apply > would be annoyed by this and hence i always show them path of my repo > where they can find it. The problem with this approach is that the complete patches never make it to the mailing lists and people have problems with connecting commits to previously posted patches. Moreover, it is *much* more convenient to me to take patches from kernel.org patchwork than from your repos, with all due respect. > So, what i may do is, post fixups and then resend > patches. So that reviewer knows what changed and others can have complete > patches too. Sure, that will work too. Thanks, Rafael
On 22 March 2013 17:41, Rafael J. Wysocki <rjw@sisk.pl> wrote: > Well, if the submitter wants to cheat, she/he certainly can this way, but > what's the benefit, honestly? If the reviewer actually notices that there are > more differences than the submitter admits to, the consequences may be quite > unpleasant for the submitter (like the rejection of any future patches, for > example). And mistakes are possible anyway (and the more patches you deal > with, the greater the chances of making a mistake are). Agreed.. I was focusing on the last part: mistakes.. People learn over time and can still commit mistakes.. So a diff is always better to see what changed. -- 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
On Friday, March 22, 2013 05:21:19 PM Viresh Kumar wrote: > On Fri, Mar 22, 2013 at 5:25 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > On Friday, March 22, 2013 07:50:54 AM Viresh Kumar wrote: > > >> Hmm... I always thought fixups are way easy to review (and i still > >> believe that's > >> true) as they just contain what got changed and so people don't have to review > >> whole patch again. > > > > They won't have to if you write in the preamble what the differences from > > previous versions are. > > You didn't get me.. How will the reviewer check if author has done what he > is saying in preamble and he hasn't broken anything new? Well, if the submitter wants to cheat, she/he certainly can this way, but what's the benefit, honestly? If the reviewer actually notices that there are more differences than the submitter admits to, the consequences may be quite unpleasant for the submitter (like the rejection of any future patches, for example). And mistakes are possible anyway (and the more patches you deal with, the greater the chances of making a mistake are). Thanks. Rafael
On Fri, Mar 22, 2013 at 12:44:40AM +0100, Rafael J. Wysocki wrote: > On Wednesday, March 20, 2013 10:59:13 AM Viresh Kumar wrote: > > On 4 March 2013 13:07, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > Currently, there can't be multiple instances of single governor_type. If we have > > > a multi-package system, where we have multiple instances of struct policy (per > > > package), we can't have multiple instances of same governor. i.e. We can't have > > > multiple instances of ondemand governor for multiple packages. > > > > > > Governors directory in sysfs is created at /sys/devices/system/cpu/cpufreq/ > > > governor-name/. Which again reflects that there can be only one instance of a > > > governor_type in the system. > > > > > > This is a bottleneck for multicluster system, where we want different packages > > > to use same governor type, but with different tunables. > > > > > > This patch uses the infrastructure provided by earlier patch and implements > > > init/exit routines for ondemand and conservative governors. > > > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > > > As discussed in other patch thread, i dropped "cpufreq: Add Kconfig option > > to enable/disable have_multiple_policies" patch and following is the fixup > > to this patch: > > > > I have queued all patches i had for 3.10 here: > > > > http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/for-3.10 > > OK, applied these to linux-pm.git/bleeding-edge. 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? Thanks, -- 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
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
On Wed, Mar 27, 2013 at 01:02:15AM +0530, Viresh Kumar wrote: > 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 > > 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; > + Hmm .. I don't think this works for both ondemand and conservative governors running at the same time . -- 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 a843855..3d83b02 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -128,6 +128,14 @@ void disable_cpufreq(void) static LIST_HEAD(cpufreq_governor_list); static DEFINE_MUTEX(cpufreq_governor_mutex); +struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy) +{ + if (cpufreq_driver->have_multiple_policies) + return &policy->kobj; + else + return cpufreq_global_kobject; +} + static struct cpufreq_policy *__cpufreq_cpu_get(unsigned int cpu, bool sysfs) { struct cpufreq_policy *data; diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 6e1abd2..805c4d3 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -107,11 +107,6 @@ struct cpufreq_policy { unsigned int policy; /* see above */ struct cpufreq_governor *governor; /* see below */ void *governor_data; - /* This should be set by init() of platforms having multiple - * clock-domains, i.e. supporting multiple policies. With this sysfs - * directories of governor would be created in cpu/cpu<num>/cpufreq/ - * directory */ - bool have_multiple_policies; struct work_struct update; /* if update_policy() needs to be * called, but you're in IRQ context */ @@ -139,15 +134,6 @@ static inline bool policy_is_shared(struct cpufreq_policy *policy) return cpumask_weight(policy->cpus) > 1; } -static inline struct kobject * -get_governor_parent_kobj(struct cpufreq_policy *policy) -{ - if (policy->have_multiple_policies) - return &policy->kobj; - else - return cpufreq_global_kobject; -} - /******************** cpufreq transition notifiers *******************/