On Wed, Mar 21, 2007 at 08:10:42PM -0800, Andrew Morton wrote:
> On Wed, 21 Mar 2007 21:00:06 -0700 Greg KH <[EMAIL PROTECTED]> wrote:
>
> > On Wed, Mar 21, 2007 at 11:51:04PM -0400, Dave Jones wrote:
> > > On Wed, Mar 21, 2007 at 08:07:53PM -0700, Greg KH wrote:
> > >
> > > > > After modprobe/rmmod cpufreq/stats directory appears but doesn't get
> > > > > removed. Should it?
> > > > Well, one can argue that those stats should never be in sysfs at all
> > > > anyway, I mean come on, a histogram in sysfs? That's, not ok.
> > >
> > > Meh, it's only a cheesy debug thing, so it's not really that big a deal
> > > imo.
> > > it could probably move to debugfs (we didn't have that when it was merged
> > > iirc)
> > > I doubt anyone really cares enough to bother though I wouldn't
> > > be averse to a patch.
> >
> > Yeah, I realize this, I'm not trying to find fault, sorry if it came
> > across that way. And yes, it should move to debugfs some day, but if it
> > does, I'll loose my "what not to put in sysfs" example I use in
> > presentations :)
> >
>
> I ain't picky, but as a short-term thing it'd be kinda nice if it didn't
> oops the kernel.
There are other symptoms to this same bug:
1. unload p4-clockmod: /sys/.../cpu0/cpufreq is removed all together
2. load p4-clockmod: /sys/.../cpu0/cpufreq appears but no 'stats' subdir
(yes, cpufreq_stats is loaded)
3. rmmod cpufreq_stats: Ooops!
Call Trace:
[<c0183f5b>] remove_dir+0x33/0xc4
[<c0184fca>] remove_files+0x1a/0x28
[<c018503b>] sysfs_remove_group+0x63/0x71
[<f898c38d>] cpufreq_stat_cpu_callback+0x51/0x8a [cpufreq_stats]
[<f898c477>] cpufreq_stats_exit+0x47/0x4b [cpufreq_stats]
[<c012f145>] sys_delete_module+0x190/0x1b7
[<c0140073>] do_wp_page+0x231/0x3e7
[<c0102e17>] syscall_call+0x7/0xb
The problem is cpufreq_stats doesn't know when a cpufreq driver is
removed and doesn't cleanup. I guess this affects any setup with
cpufreq_stats.
The attached patch seems to solve both symptoms and yes... it's quite
invasive as it introduce one more cpufreq policy notification (REMOVED).
BTW: the patch is against .21-rc4-mm1 but applies with some fuzz to
2.6.20 too
diff -rup linux-2.6.20/drivers/cpufreq/cpufreq.c
linux-2.6.20.dirty/drivers/cpufreq/cpufreq.c
--- linux-2.6.20/drivers/cpufreq/cpufreq.c 2007-03-22 17:00:38.000000000
+0100
+++ linux-2.6.20.dirty/drivers/cpufreq/cpufreq.c 2007-03-22
16:51:09.000000000 +0100
@@ -989,6 +989,10 @@ static int __cpufreq_remove_dev (struct
unlock_policy_rwsem_write(cpu);
+ /* notify of policy cancellation */
+ blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
+ CPUFREQ_REMOVE, data);
+
kobject_unregister(&data->kobj);
kobject_put(&data->kobj);
diff -rup linux-2.6.20/drivers/cpufreq/cpufreq_stats.c
linux-2.6.20.dirty/drivers/cpufreq/cpufreq_stats.c
--- linux-2.6.20/drivers/cpufreq/cpufreq_stats.c 2007-03-22
17:00:38.000000000 +0100
+++ linux-2.6.20.dirty/drivers/cpufreq/cpufreq_stats.c 2007-03-22
17:06:24.000000000 +0100
@@ -257,18 +257,23 @@ 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;
- if (val != CPUFREQ_NOTIFY)
- return 0;
- table = cpufreq_frequency_get_table(cpu);
- if (!table)
- return 0;
- if ((ret = cpufreq_stats_create_table(policy, table)))
- return ret;
- return 0;
+ switch (val) {
+ case CPUFREQ_NOTIFY:
+ table = cpufreq_frequency_get_table(cpu);
+ if (!table)
+ break;
+ ret = cpufreq_stats_create_table(policy, table);
+ break;
+
+ case CPUFREQ_REMOVE:
+ cpufreq_stats_free_table(cpu);
+ break;
+ }
+ return ret;
}
static int
@@ -371,8 +376,7 @@ __exit cpufreq_stats_exit(void)
CPUFREQ_TRANSITION_NOTIFIER);
unregister_hotcpu_notifier(&cpufreq_stat_cpu_notifier);
for_each_online_cpu(cpu) {
- cpufreq_stat_cpu_callback(&cpufreq_stat_cpu_notifier,
- CPU_DEAD, (void *)(long)cpu);
+ cpufreq_stats_free_table(cpu);
}
}
--- linux-2.6.20/include/linux/cpufreq.h 2007-03-22 17:00:47.000000000
+0100
+++ linux-2.6.20.dirty/include/linux/cpufreq.h 2007-03-22 16:10:37.000000000
+0100
@@ -96,6 +96,7 @@ struct cpufreq_policy {
#define CPUFREQ_ADJUST (0)
#define CPUFREQ_INCOMPATIBLE (1)
#define CPUFREQ_NOTIFY (2)
+#define CPUFREQ_REMOVE (3)
#define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
#define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed coordination */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
