[PATCH] fix cpufreq_stats attrs removal

[Mattia Dongili]

On Wed, Mar 21, 2007 at 08:10:42PM -0800, Andrew Morton wrote:
> On Wed, 21 Mar 2007 21:00:06 -0700 Greg KH <gregkh@xxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/