LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Oops after cd /sys/.../cpufreq/; rmmod; cat stats/time_in_state
@ 2007-03-19 15:30 Alexey Dobriyan
  2007-03-19 20:41 ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Alexey Dobriyan @ 2007-03-19 15:30 UTC (permalink / raw)
  To: linux-kernel; +Cc: davej, gregkh, akpm

Steps to reproduce:

	# modprobe p4-clockmod
	$ cd /sys/devices/system/cpu/cpu0/cpufreq/
	# rmmod p4-clockmod
	$ cat stats/time_in_state
	Segmentation fault

p4-clockmod: P4/Xeon(TM) CPU On-Demand Clock Modulation available
BUG: unable to handle kernel paging request at virtual address 6b6b6b6f
 printing eip:
c01906b4
*pde = 00000000
Oops: 0000 [#1]
PREEMPT
Modules linked in: speedstep_lib ohci_hcd af_packet e1000 ehci_hcd uhci_hcd usbcore xfs
CPU:    0
EIP:    0060:[<c01906b4>]    Not tainted VLI
EFLAGS: 00010202   (2.6.21-rc4-5851fadce8824d5d4b8fd02c22ae098401f6489e #2)
EIP is at sysfs_open_file+0xd3/0x259
eax: 6b6b6b6b   ebx: 00000000   ecx: f371b000   edx: 00000000
esi: c03a7330   edi: f35c8408   ebp: f371c9b0   esp: f371bee0
ds: 007b   es: 007b   fs: 00d8  gs: 0033  ss: 0068
Process cat (pid: 6876, ti=f371b000 task=c192f550 task.ti=f371b000)
Stack: f3713b2c c0393400 00000000 f371c9b0 00000000 f3713b2c c01905e1 c0158575
       c18d3208 f71c4cd8 00000000 f371c9b0 00008000 f371bf38 ffffff9c c0158702
       f371c9b0 00000000 00000000 c0158744 00000000 f371bf38 f71c4cd8 c18d3208
Call Trace:
 [<c01905e1>] sysfs_open_file+0x0/0x259
 [<c0158575>] __dentry_open+0xec/0x1ec
 [<c0158702>] nameidata_to_filp+0x31/0x3a
 [<c0158744>] do_filp_open+0x39/0x40
 [<c0158478>] get_unused_fd+0xa1/0xb2
 [<c02d4cde>] _spin_unlock+0x25/0x3b
 [<c0158478>] get_unused_fd+0xa1/0xb2
 [<c0158785>] do_sys_open+0x3a/0x6d
 [<c01587f3>] sys_open+0x1c/0x20
 [<c0103e72>] sysenter_past_esp+0x5f/0x99
 =======================
Code: 00 f0 ff ff 8b 40 08 a8 08 0f 85 95 01 00 00 bb ed ff ff ff 8b 44 24 08 85 c0 0f 84 f8 00 00 00 8b 47 28 85 c0 0f 84 34 01 00 00 <8b> 40 04 85 c0 0f 84 29 01 00 00 8b 58 04 85 db 0f 84 0f 01 00
EIP: [<c01906b4>] sysfs_open_file+0xd3/0x259 SS:ESP 0068:f371bee0
Slab corruption: start=f35c83bc, len=256
Redzone: 0x5a2cf071/0x5a2cf071.
Last user: [<c0181d54>](load_elf_binary+0x161b/0x1ac7)
060: 6b 6b 6b 6b 6c 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Prev obj: start=f35c82b0, len=256
Redzone: 0x170fc2a5/0x170fc2a5.
Last user: [<c0180758>](load_elf_binary+0x1f/0x1ac7)
000: 7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00
010: 02 00 03 00 01 00 00 00 b0 85 04 08 34 00 00 00
Next obj: start=f35c84c8, len=256
Redzone: 0x170fc2a5/0x170fc2a5.
Last user: [<c01807ea>](load_elf_binary+0xb1/0x1ac7)
000: 06 00 00 00 34 00 00 00 34 80 04 08 34 80 04 08
010: 00 01 00 00 00 01 00 00 05 00 00 00 04 00 00 00


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Oops after cd /sys/.../cpufreq/; rmmod; cat stats/time_in_state
  2007-03-19 15:30 Oops after cd /sys/.../cpufreq/; rmmod; cat stats/time_in_state Alexey Dobriyan
@ 2007-03-19 20:41 ` Greg KH
  2007-03-20 10:06   ` Alexey Dobriyan
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2007-03-19 20:41 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: linux-kernel, davej, akpm

On Mon, Mar 19, 2007 at 06:30:13PM +0300, Alexey Dobriyan wrote:
> Steps to reproduce:
> 
> 	# modprobe p4-clockmod
> 	$ cd /sys/devices/system/cpu/cpu0/cpufreq/
> 	# rmmod p4-clockmod
> 	$ cat stats/time_in_state
> 	Segmentation fault

Has this always happened?  Or is it new?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Oops after cd /sys/.../cpufreq/; rmmod; cat stats/time_in_state
  2007-03-19 20:41 ` Greg KH
@ 2007-03-20 10:06   ` Alexey Dobriyan
  2007-03-22  3:07     ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Alexey Dobriyan @ 2007-03-20 10:06 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, davej, akpm

On Mon, Mar 19, 2007 at 01:41:25PM -0700, Greg KH wrote:
> On Mon, Mar 19, 2007 at 06:30:13PM +0300, Alexey Dobriyan wrote:
> > Steps to reproduce:
> >
> > 	# modprobe p4-clockmod
> > 	$ cd /sys/devices/system/cpu/cpu0/cpufreq/
> > 	# rmmod p4-clockmod
> > 	$ cat stats/time_in_state
> > 	Segmentation fault
>
> Has this always happened?  Or is it new?

I've checked 2.6.17 and up and it happens too.

Some .config peculiarities:

	CONFIG_CPU_FREQ=y
	CONFIG_CPU_FREQ_STAT=y
	CONFIG_X86_P4_CLOCKMOD=m

After modprobe/rmmod cpufreq/stats directory appears but doesn't get
removed. Should it?

/sys/devices/system/cpu/cpu0/cpufreq $ ls -lR
.:
total 0
drwxr-xr-x 2 root root 0 2007-03-20 12:59 stats

./stats:
total 0
-r--r--r-- 1 root root 4096 2007-03-20 12:59 time_in_state
-r--r--r-- 1 root root 4096 2007-03-20 12:59 total_trans


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Oops after cd /sys/.../cpufreq/; rmmod; cat stats/time_in_state
  2007-03-20 10:06   ` Alexey Dobriyan
@ 2007-03-22  3:07     ` Greg KH
  2007-03-22  3:51       ` Dave Jones
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2007-03-22  3:07 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: linux-kernel, davej, akpm

On Tue, Mar 20, 2007 at 01:06:34PM +0300, Alexey Dobriyan wrote:
> On Mon, Mar 19, 2007 at 01:41:25PM -0700, Greg KH wrote:
> > On Mon, Mar 19, 2007 at 06:30:13PM +0300, Alexey Dobriyan wrote:
> > > Steps to reproduce:
> > >
> > > 	# modprobe p4-clockmod
> > > 	$ cd /sys/devices/system/cpu/cpu0/cpufreq/
> > > 	# rmmod p4-clockmod
> > > 	$ cat stats/time_in_state
> > > 	Segmentation fault
> >
> > Has this always happened?  Or is it new?
> 
> I've checked 2.6.17 and up and it happens too.
> 
> Some .config peculiarities:
> 
> 	CONFIG_CPU_FREQ=y
> 	CONFIG_CPU_FREQ_STAT=y
> 	CONFIG_X86_P4_CLOCKMOD=m
> 
> After modprobe/rmmod cpufreq/stats directory appears but doesn't get
> removed. Should it?

Yes.

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.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Oops after cd /sys/.../cpufreq/; rmmod; cat stats/time_in_state
  2007-03-22  3:07     ` Greg KH
@ 2007-03-22  3:51       ` Dave Jones
  2007-03-22  4:00         ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: Dave Jones @ 2007-03-22  3:51 UTC (permalink / raw)
  To: Greg KH; +Cc: Alexey Dobriyan, linux-kernel, akpm

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.
To the best of my knowledge, nothing in userspace is relying on that stuff
being present (it'd have to cope with it not being there anyway given its
optional).

	Dave

-- 
http://www.codemonkey.org.uk

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Oops after cd /sys/.../cpufreq/; rmmod; cat stats/time_in_state
  2007-03-22  3:51       ` Dave Jones
@ 2007-03-22  4:00         ` Greg KH
  2007-03-22  4:10           ` Andrew Morton
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2007-03-22  4:00 UTC (permalink / raw)
  To: Dave Jones, Alexey Dobriyan, linux-kernel, akpm

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 :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Oops after cd /sys/.../cpufreq/; rmmod; cat stats/time_in_state
  2007-03-22  4:00         ` Greg KH
@ 2007-03-22  4:10           ` Andrew Morton
  2007-03-22 17:02             ` [PATCH] fix cpufreq_stats attrs removal Mattia Dongili
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2007-03-22  4:10 UTC (permalink / raw)
  To: Greg KH; +Cc: Dave Jones, Alexey Dobriyan, linux-kernel

On Wed, 21 Mar 2007 21:00:06 -0700 Greg KH <gregkh@suse.de> 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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] fix cpufreq_stats attrs removal
  2007-03-22  4:10           ` Andrew Morton
@ 2007-03-22 17:02             ` Mattia Dongili
  2007-03-22 17:14               ` Dave Jones
                                 ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Mattia Dongili @ 2007-03-22 17:02 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Greg KH, Dave Jones, Alexey Dobriyan, linux-kernel

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@suse.de> 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 */

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fix cpufreq_stats attrs removal
  2007-03-22 17:02             ` [PATCH] fix cpufreq_stats attrs removal Mattia Dongili
@ 2007-03-22 17:14               ` Dave Jones
  2007-03-22 17:43               ` Mattia Dongili
  2007-03-30  8:09               ` Alexey Dobriyan
  2 siblings, 0 replies; 13+ messages in thread
From: Dave Jones @ 2007-03-22 17:14 UTC (permalink / raw)
  To: Andrew Morton, Greg KH, Alexey Dobriyan, linux-kernel

On Thu, Mar 22, 2007 at 06:02:01PM +0100, Mattia Dongili wrote:

 > 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

Alternatively, as it's just debug functionality, we could just
mark it unsafe for rmmod, which is nastier, but a lot less invasive
to the non-debug code.

	Dave

-- 
http://www.codemonkey.org.uk

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fix cpufreq_stats attrs removal
  2007-03-22 17:02             ` [PATCH] fix cpufreq_stats attrs removal Mattia Dongili
  2007-03-22 17:14               ` Dave Jones
@ 2007-03-22 17:43               ` Mattia Dongili
  2007-03-22 17:50                 ` Dave Jones
  2007-03-30  8:09               ` Alexey Dobriyan
  2 siblings, 1 reply; 13+ messages in thread
From: Mattia Dongili @ 2007-03-22 17:43 UTC (permalink / raw)
  To: Andrew Morton, Greg KH, Dave Jones, Alexey Dobriyan, linux-kernel

On Thu, Mar 22, 2007 at 06:02:01PM +0100, Mattia Dongili wrote:

forgot... (should I resend the whole thing?)

Signed-off-by: Mattia Dongili <malattia@linux.it>
-- 
mattia
:wq!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fix cpufreq_stats attrs removal
  2007-03-22 17:43               ` Mattia Dongili
@ 2007-03-22 17:50                 ` Dave Jones
  0 siblings, 0 replies; 13+ messages in thread
From: Dave Jones @ 2007-03-22 17:50 UTC (permalink / raw)
  To: Andrew Morton, Greg KH, Alexey Dobriyan, linux-kernel

On Thu, Mar 22, 2007 at 06:43:21PM +0100, Mattia Dongili wrote:
 > On Thu, Mar 22, 2007 at 06:02:01PM +0100, Mattia Dongili wrote:
 > 
 > forgot... (should I resend the whole thing?)
 > 
 > Signed-off-by: Mattia Dongili <malattia@linux.it>

Nah, I'll cut-n-paste.
Ignore my other mail btw, I wasn't thinking straight.
Marking the stats module unsafe for removal won't prevent
the race when we remove _other_ modules :-)

	Dave

-- 
http://www.codemonkey.org.uk

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fix cpufreq_stats attrs removal
  2007-03-22 17:02             ` [PATCH] fix cpufreq_stats attrs removal Mattia Dongili
  2007-03-22 17:14               ` Dave Jones
  2007-03-22 17:43               ` Mattia Dongili
@ 2007-03-30  8:09               ` Alexey Dobriyan
  2007-03-30 18:16                 ` Mattia Dongili
  2 siblings, 1 reply; 13+ messages in thread
From: Alexey Dobriyan @ 2007-03-30  8:09 UTC (permalink / raw)
  To: Andrew Morton, Mattia Dongili; +Cc: Greg KH, Dave Jones, linux-kernel

On Thu, Mar 22, 2007 at 06:02:01PM +0100, Mattia Dongili wrote:
> On Wed, Mar 21, 2007 at 08:10:42PM -0800, Andrew Morton wrote:
> > 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

Also, it doesn't work.

/sys/*/cpufreq/stats dir stays if you cd into it _before_ rmmod.

	# modprobe p4-clockmod
	$ /sys/devices/system/cpu/cpu0/cpufreq/stats
	# rmmod p4-clockmod
	$ cat time_in_state

p4-clockmod: P4/Xeon(TM) CPU On-Demand Clock Modulation available
BUG: unable to handle kernel paging request at virtual address 6b6b6b6f
 printing eip:
c01955e1
*pde = 00000000
Oops: 0000 [#1]
last sysfs file: devices/system/cpu/cpu0/cpufreq/stats/time_in_state
Modules linked in: speedstep_lib ohci_hcd af_packet e1000 ehci_hcd uhci_hcd usbcore
CPU:    0
EIP:    0060:[<c01955e1>]    Not tainted VLI
EFLAGS: 00010202   (2.6.21-rc5-mm1 #2)
EIP is at sysfs_open_file+0xb6/0x26f
eax: 6b6b6b6b   ebx: c03b3cb0   ecx: 00000000   edx: f732772c
esi: 00000000   edi: c0681400   ebp: f36fad10   esp: f3595ee0
ds: 007b   es: 007b   fs: 00d8  gs: 0033  ss: 0068
Process cat (pid: 7545, ti=f3594000 task=f3773510 task.ti=f3594000)
Stack: 00001000 f376f26c f732772c f376f26c f36fad10 f376f26c f3595f38 c019552b 
       c015ced8 c18d3190 f3746a18 f36fad10 00008000 f3595f38 ffffff9c c015d052 
       f36fad10 00000000 00000000 c015d094 00000000 f3595f38 f3746a18 c18d3190 
Call Trace:
 [<c019552b>] sysfs_open_file+0x0/0x26f
 [<c015ced8>] __dentry_open+0xa4/0x191
 [<c015d052>] nameidata_to_filp+0x31/0x3a
 [<c015d094>] do_filp_open+0x39/0x40
 [<c015ce23>] get_unused_fd+0xa1/0xb2
 [<c02db41f>] _spin_unlock+0x14/0x1c
 [<c015ce23>] get_unused_fd+0xa1/0xb2
 [<c015d0d5>] do_sys_open+0x3a/0x6d
 [<c015d143>] sys_open+0x1c/0x20
 [<c0103c96>] sysenter_past_esp+0x5f/0x99
 =======================
INFO: lockdep is turned off.
Code: 0f 85 24 01 00 00 8b 43 04 85 c0 74 0f 83 38 02 0f 84 c4 01 00 00 ff 80 80 01 00 00 8b 54 24 08 8b 42 28 85 c0 0f 84 62 01 00 00 <8b> 40 04 85 c0 0f 84 57 01 00 00 8b 40 04 89 44 24 0c 8b 74 24 
EIP: [<c01955e1>] sysfs_open_file+0xb6/0x26f SS:ESP 0068:f3595ee0
Slab corruption: start=f73276e0, len=256
Redzone: 0x5a2cf071/0x5a2cf071.
Last user: [<c0185810>](load_elf_binary+0xa79/0x1a19)
060: 6b 6b 6b 6b 6c 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Prev obj: start=f73275d4, len=256
Redzone: 0x5a2cf071/0x5a2cf071.
Last user: [<c01864fb>](load_elf_binary+0x1764/0x1a19)
000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Next obj: start=f73277ec, len=256
Redzone: 0x5a2cf071/0x5a2cf071.
Last user: [<c01864fb>](load_elf_binary+0x1764/0x1a19)
000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b

> --- 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 */


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fix cpufreq_stats attrs removal
  2007-03-30  8:09               ` Alexey Dobriyan
@ 2007-03-30 18:16                 ` Mattia Dongili
  0 siblings, 0 replies; 13+ messages in thread
From: Mattia Dongili @ 2007-03-30 18:16 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: Andrew Morton, Greg KH, Dave Jones, linux-kernel

On Fri, Mar 30, 2007 at 12:09:03PM +0400, Alexey Dobriyan wrote:
> On Thu, Mar 22, 2007 at 06:02:01PM +0100, Mattia Dongili wrote:
> > On Wed, Mar 21, 2007 at 08:10:42PM -0800, Andrew Morton wrote:
> > > 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
> 
> Also, it doesn't work.

hmmm... odd. I did test that case. Also, using .21-rc5-mm3 (which
has that patch applied) and acpi-cpufreq (I don't have that P4 handy
now) I actually hit a WARN_ON in kref_get(), but no oops:

# rmmod acpi-cpufreq

[ 1839.895632] acpi-cpufreq: acpi_cpufreq_exit
[ 1839.895637] cpufreq-core: unregistering driver acpi-cpufreq
[ 1839.895642] cpufreq-core: unregistering CPU 0
[ 1839.895649] cpufreq-core: __cpufreq_governor for CPU 0, event 2
[ 1839.895661] cpufreq-stats: removing tables
[ 1839.895714] cpufreq-core: last reference is dropped
[ 1839.895716] cpufreq-core: waiting for dropping of refcount
[ 1839.895718] cpufreq-core: wait complete
[ 1839.895720] acpi-cpufreq: acpi_cpufreq_cpu_exit
[ 1839.895723] freq-table: clearing show_table for cpu 0
[ 1839.895726] cpufreq-core: unregistering CPU 1
[ 1839.895728] cpufreq-core: __cpufreq_governor for CPU 1, event 2
[ 1839.895731] cpufreq-stats: removing tables    <------- dprintk on CPUFREQ_REMOVE
[ 1839.895741] cpufreq-core: last reference is dropped
[ 1839.895744] cpufreq-core: waiting for dropping of refcount
[ 1839.895746] cpufreq-core: wait complete
[ 1839.895747] acpi-cpufreq: acpi_cpufreq_cpu_exit
[ 1839.895749] freq-table: clearing show_table for cpu 1

# cat time_in_state

[ 1853.932509] BUG: at lib/kref.c:32 kref_get()
[ 1853.932567]  [<c0104b7a>] show_trace_log_lvl+0x1a/0x30
[ 1853.932676]  [<c0105705>] show_trace+0x12/0x14
[ 1853.932778]  [<c010575e>] dump_stack+0x16/0x18
[ 1853.932880]  [<c01dcfba>] kref_get+0x37/0x40
[ 1853.932986]  [<c01dc415>] kobject_get+0x15/0x1b
[ 1853.933087]  [<c019ff0b>] sysfs_open_file+0x40/0x209
[ 1853.933189]  [<c01693bc>] __dentry_open+0xc2/0x179
[ 1853.933293]  [<c01694f5>] nameidata_to_filp+0x27/0x38
[ 1853.933395]  [<c0169539>] do_filp_open+0x33/0x3b
[ 1853.933498]  [<c0169584>] do_sys_open+0x43/0xc7
[ 1853.933600]  [<c0169640>] sys_open+0x1c/0x1e
[ 1853.933700]  [<c0103d24>] sysenter_past_esp+0x5d/0x81
[ 1853.933799]  =======================
[ 1853.934330] cpufreq-core: last reference is dropped

-- 
mattia
:wq!

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2007-03-30 18:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-19 15:30 Oops after cd /sys/.../cpufreq/; rmmod; cat stats/time_in_state Alexey Dobriyan
2007-03-19 20:41 ` Greg KH
2007-03-20 10:06   ` Alexey Dobriyan
2007-03-22  3:07     ` Greg KH
2007-03-22  3:51       ` Dave Jones
2007-03-22  4:00         ` Greg KH
2007-03-22  4:10           ` Andrew Morton
2007-03-22 17:02             ` [PATCH] fix cpufreq_stats attrs removal Mattia Dongili
2007-03-22 17:14               ` Dave Jones
2007-03-22 17:43               ` Mattia Dongili
2007-03-22 17:50                 ` Dave Jones
2007-03-30  8:09               ` Alexey Dobriyan
2007-03-30 18:16                 ` Mattia Dongili

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).