Re: [PATCH] fix cpufreq_stats attrs removal

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

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

Relevant Pages

  • Re: 2.6.19-rc2-mm1
    ... testing NMI watchdog ... ... The x86_64 nmi watchdog handling looks rather complex. ... I don't think trace all cpus ... BUG: ...
    (Linux-Kernel)
  • [BUG] 2.6.27-rc1 in ext3_find_entry
    ... in the middle of running git; I forget exactly what I was doing. ... BUG: unable to handle kernel paging request at ffff88004fc00006 ... Call Trace: ...
    (Linux-Kernel)
  • Re: cpu load balancing problem on smp
    ... this is not an irq distribution that irqbalance should give ... since this only happens when he adds more iptables rules, ... BUG: ... Call Trace: ...
    (Linux-Kernel)
  • [BUG] 2.6.29-rc* QinQ vlan trunking regression
    ... another vlan, so called vlan trunking or QinQ. ... source of the bug. ... $ git revert cc883d16c3b7434c7da2c45b54a49c2a99e83db7 ... there is no trace of this bug. ...
    (Linux-Kernel)
  • Re: [PATCH 2/6] tracing: increase size of number of possible events
    ... rmmod trace-events-sample.ko ... The bug won't be triggered with your latest changes, ... to do with the debugfs patch, ... ring buffer the other module's callback will be called with the previous ...
    (Linux-Kernel)