v2.6.28-rc7: error in panic code? (NULL pointer dereference at 0000004c)
Subsystems (2)
Hi,
With such a patch:
init/main.c +2 -0
@@ -465,6 +465,8 @@ static void noinline __init_refok rest_init(void) {int pid; + *(char *)NULL =0 ; +kernel_thread (kernel_init,NULL ,CLONE_FS |CLONE_SIGHAND );numa_default_policy (); pid =kernel_thread (kthreadd,NULL ,CLONE_FS |CLONE_FILES );
...I would expect a page fault and that's that. So panic() is called, but it causes a new page fault somewhere. Here is the log:
(this part is correct and expected)
[ 0.031003] BUG: unable to handle kernel NULL pointer dereference at 00000000 [ 0.033997] IP: [<c13b448b>] rest_init+0xf/0x57 [ 0.035997] *pde = 00000000 [ 0.037289] Oops: 0002 [#1] SMP [ 0.037994] last sysfs file: [ 0.037994] Modules linked in: [ 0.037994] [ 0.037994] Pid: 0, comm: swapper Not tainted (2.6.28-rc7 #181) 945P-A [ 0.037994] EIP: 0060:[<c13b448b>] EFLAGS: 00010246 CPU: 0 [ 0.037994] EIP is at rest_init+0xf/0x57 [ 0.037994] EAX: c16631e3 EBX: 00000040 ECX: 00000a00 EDX: 00000000 [ 0.037994] ESI: 00099800 EDI: c160a000 EBP: c165dfd0 ESP: c165dfd0 [ 0.037994] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 [ 0.037994] Process swapper (pid: 0, ti=c165c000 task=c156e334 task.ti=c165c000) [ 0.037994] Stack: [ 0.037994] c165dfe0 c16637af c1691768 00000000 c165dff8 c1663080 0175a000 00000000 [ 0.037994] c14ceaae 00020800 01b7e003 00000000 [ 0.037994] Call Trace: [ 0.037994] [<c16637af>] ? start_kernel+0x2a2/0x2a7 [ 0.037994] [<c1663080>] ? __init_begin+0x80/0x88 [ 0.037994] Code: 00 8b 43 04 8d 56 04 89 50 04 89 46 04 8d 43 04 89 46 08 89 53 04 fe 03 5b 5e 5d c3 55 b9 00 0a 00 00 8 9 e5 31 d2 b8 e3 31 66 c1 <c6> 05 00 00 00 00 00 e8 00 df c4 ff b9 00 06 00 00 31 d2 b8 af [ 0.037994] EIP: [<c13b448b>] rest_init+0xf/0x57 SS:ESP 0068:c165dfd0 [ 0.038004] ---[ end trace 4eaa2a86a8e2da22 ]--- [ 0.038998] Kernel panic - not syncing: Attempted to kill the idle task!
And now the unexpected part:
[ 0.039999] Rebooting in 10 seconds..<1>BUG: unable to handle kernel NULL pointer dereference at 0000004c [ 0.040993] IP: [<c13b41dc>] klist_next+0x10/0x8d [ 0.040993] *pde = 00000000 [ 0.040993] Oops: 0000 [#2] SMP [ 0.040993] last sysfs file: [ 0.040993] Modules linked in: [ 0.040993] [ 0.040993] Pid: 0, comm: swapper Tainted: G D (2.6.28-rc7 #181) 945P-A [ 0.040993] EIP: 0060:[<c13b41dc>] EFLAGS: 00010286 CPU: 0 [ 0.040993] EIP is at klist_next+0x10/0x8d [ 0.040993] EAX: 0000003c EBX: c165dd60 ECX: 00000000 EDX: c165dd60 [ 0.040993] ESI: c165dd60 EDI: 00000000 EBP: c165dd58 ESP: c165dd48 [ 0.040993] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 [ 0.040993] Process swapper (pid: 0, ti=c165c000 task=c156e334 task.ti=c165c000) [ 0.040993] Stack: [ 0.040993] c1026a97 c165dd60 c165dd60 00000000 c165dd74 c11be196 0000003c 00000000 [ 0.040993] c13dbde0 00001078 00000100 c165dd84 c114ed26 c114e458 c13dbde0 c165dd9c [ 0.040993] c1152546 ffffffff c13dbde0 00002710 0000000b c165ddac c115259a ffffffff [ 0.040993] Call Trace: [ 0.040993] [<c1026a97>] ? release_console_sem+0x16c/0x199 [ 0.040993] [<c11be196>] ? bus_find_device+0x4e/0x6e [ 0.040993] [<c114ed26>] ? no_pci_devices+0x17/0x2d [ 0.040993] [<c114e458>] ? find_anything+0x0/0xa [ 0.040993] [<c1152546>] ? pci_get_subsys+0x15/0x5b [ 0.040993] [<c115259a>] ? pci_get_device+0xe/0x10 [ 0.040993] [<c1013342>] ? mach_reboot_fixups+0x27/0x3c [ 0.040993] [<c100fdfb>] ? native_machine_emergency_restart+0x3e/0xd7 [ 0.040993] [<c100fc60>] ? machine_emergency_restart+0x9/0xb [ 0.040993] [<c1032b4b>] ? emergency_restart+0x8/0xa [ 0.040993] [<c13cf607>] ? panic+0xb9/0xd6 [ 0.040993] [<c1028ee0>] ? do_exit+0x5b/0x740 [ 0.040993] [<c13cf633>] ? printk+0xf/0x11 [ 0.040993] [<c10263df>] ? print_oops_end_marker+0x1e/0x23 [ 0.040993] [<c13d1c1e>] ? oops_end+0x7f/0x87 [ 0.040993] [<c1005a08>] ? die+0x5b/0x63 [ 0.040993] [<c13d3061>] ? do_page_fault+0x581/0x66f [ 0.040993] [<c103a537>] ? sched_clock_cpu+0x136/0x142 [ 0.040993] [<c103a537>] ? sched_clock_cpu+0x136/0x142 [ 0.040993] [<c1039165>] ? ktime_get+0x13/0x2f [ 0.040993] [<c103a562>] ? sched_clock_idle_sleep_event+0xe/0x10 [ 0.040993] [<c102a86f>] ? __do_softirq+0x119/0x121 [ 0.040993] [<c116e282>] ? acpi_hw_low_level_read+0x3b/0x68 [ 0.040993] [<c116e34f>] ? acpi_hw_register_read+0xa0/0x112 [ 0.040993] [<c116e4f0>] ? acpi_get_register_unlocked+0x2c/0x48 [ 0.040993] [<c1161aa7>] ? acpi_os_release_lock+0x8/0xa [ 0.040993] [<c116e67b>] ? acpi_get_register+0x2d/0x34 [ 0.040993] [<c13d2ae0>] ? do_page_fault+0x0/0x66f [ 0.040993] [<c13d13da>] ? error_code+0x72/0x78 [ 0.040993] [<c16631e3>] ? kernel_init+0x0/0x148 [ 0.040993] [<c13b448b>] ? rest_init+0xf/0x57 [ 0.040993] [<c16637af>] ? start_kernel+0x2a2/0x2a7 [ 0.040993] [<c1663080>] ? __init_begin+0x80/0x88 [ 0.040993] Code: 89 4a 04 74 08 8d 41 0c e8 fa 04 d9 ff 5d c3 55 31 c9 89 e5 e8 e0 ff ff ff 5d c3 55 89 e5 57 56 89 c6 5 3 83 ec 04 8b 00 8b 7e 04 <8b> 50 10 89 55 f0 e8 7b cf 01 00 85 ff 74 23 8b 47 04 ba ec 42 [ 0.040993] EIP: [<c13b41dc>] klist_next+0x10/0x8d SS:ESP 0068:c165dd48
I know this is not much to fuzz about since it was artificially induced with the NULL pointer dereference, but what if such an error (a real one) made it into the kernel, it could scroll away the real oops. Anyway -- to reproduce, apply the patch and boot with panic=10 (1 also works). Thanks for the attention,
Vegard
Signature · 4 lines
"The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
Quoted 119 lines · Vegard Nossum
Hi,
With such a patch:
init/main.c +2
-0@@ -465,6 +465,8 @@ static void noinline __init_refok rest_init(void) {int pid; + *(char *)NULL =0 ; +kernel_thread (kernel_init,NULL ,CLONE_FS |CLONE_SIGHAND );numa_default_policy (); pid =kernel_thread (kthreadd,NULL ,CLONE_FS |CLONE_FILES );...I would expect a page fault and that's that. So
panic()is called, but it causes a new page fault somewhere. Here is the log:(this part is correct and expected)
[ 0.031003] BUG: unable to handle kernel NULL pointer dereference at 00000000 [ 0.033997] IP: [<c13b448b>] rest_init+0xf/0x57 [ 0.035997] *pde = 00000000 [ 0.037289] Oops: 0002 [#1] SMP [ 0.037994] last sysfs file: [ 0.037994] Modules linked in: [ 0.037994] [ 0.037994] Pid: 0, comm: swapper Not tainted (2.6.28-rc7 #181) 945P-A [ 0.037994] EIP: 0060:[<c13b448b>] EFLAGS: 00010246 CPU: 0 [ 0.037994] EIP is at rest_init+0xf/0x57 [ 0.037994] EAX: c16631e3 EBX: 00000040 ECX: 00000a00 EDX: 00000000 [ 0.037994] ESI: 00099800 EDI: c160a000 EBP: c165dfd0 ESP: c165dfd0 [ 0.037994] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 [ 0.037994] Process swapper (pid: 0, ti=c165c000 task=c156e334 task.ti=c165c000) [ 0.037994] Stack: [ 0.037994] c165dfe0 c16637af c1691768 00000000 c165dff8 c1663080 0175a000 00000000 [ 0.037994] c14ceaae 00020800 01b7e003 00000000 [ 0.037994] Call Trace: [ 0.037994] [<c16637af>] ? start_kernel+0x2a2/0x2a7 [ 0.037994] [<c1663080>] ? __init_begin+0x80/0x88 [ 0.037994] Code: 00 8b 43 04 8d 56 04 89 50 04 89 46 04 8d 43 04 89 46 08 89 53 04 fe 03 5b 5e 5d c3 55 b9 00 0a 00 00 8 9 e5 31 d2 b8 e3 31 66 c1 <c6> 05 00 00 00 00 00 e8 00 df c4 ff b9 00 06 00 00 31 d2 b8 af [ 0.037994] EIP: [<c13b448b>] rest_init+0xf/0x57 SS:ESP 0068:c165dfd0 [ 0.038004] ---[ end trace 4eaa2a86a8e2da22 ]--- [ 0.038998] Kernel panic - not syncing: Attempted to kill the idle task!And now the unexpected part:
[ 0.039999] Rebooting in 10 seconds..<1>BUG: unable to handle kernel NULL pointer dereference at 0000004c [ 0.040993] IP: [<c13b41dc>] klist_next+0x10/0x8d [ 0.040993] *pde = 00000000 [ 0.040993] Oops: 0000 [#2] SMP [ 0.040993] last sysfs file: [ 0.040993] Modules linked in: [ 0.040993] [ 0.040993] Pid: 0, comm: swapper Tainted: G D (2.6.28-rc7 #181) 945P-A [ 0.040993] EIP: 0060:[<c13b41dc>] EFLAGS: 00010286 CPU: 0 [ 0.040993] EIP is at klist_next+0x10/0x8d [ 0.040993] EAX: 0000003c EBX: c165dd60 ECX: 00000000 EDX: c165dd60 [ 0.040993] ESI: c165dd60 EDI: 00000000 EBP: c165dd58 ESP: c165dd48 [ 0.040993] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 [ 0.040993] Process swapper (pid: 0, ti=c165c000 task=c156e334 task.ti=c165c000) [ 0.040993] Stack: [ 0.040993] c1026a97 c165dd60 c165dd60 00000000 c165dd74 c11be196 0000003c 00000000 [ 0.040993] c13dbde0 00001078 00000100 c165dd84 c114ed26 c114e458 c13dbde0 c165dd9c [ 0.040993] c1152546 ffffffff c13dbde0 00002710 0000000b c165ddac c115259a ffffffff [ 0.040993] Call Trace: [ 0.040993] [<c1026a97>] ? release_console_sem+0x16c/0x199 [ 0.040993] [<c11be196>] ? bus_find_device+0x4e/0x6e [ 0.040993] [<c114ed26>] ? no_pci_devices+0x17/0x2d [ 0.040993] [<c114e458>] ? find_anything+0x0/0xa [ 0.040993] [<c1152546>] ? pci_get_subsys+0x15/0x5b [ 0.040993] [<c115259a>] ? pci_get_device+0xe/0x10 [ 0.040993] [<c1013342>] ? mach_reboot_fixups+0x27/0x3c [ 0.040993] [<c100fdfb>] ? native_machine_emergency_restart+0x3e/0xd7 [ 0.040993] [<c100fc60>] ? machine_emergency_restart+0x9/0xb [ 0.040993] [<c1032b4b>] ? emergency_restart+0x8/0xa [ 0.040993] [<c13cf607>] ? panic+0xb9/0xd6 [ 0.040993] [<c1028ee0>] ? do_exit+0x5b/0x740 [ 0.040993] [<c13cf633>] ? printk+0xf/0x11 [ 0.040993] [<c10263df>] ? print_oops_end_marker+0x1e/0x23 [ 0.040993] [<c13d1c1e>] ? oops_end+0x7f/0x87 [ 0.040993] [<c1005a08>] ? die+0x5b/0x63 [ 0.040993] [<c13d3061>] ? do_page_fault+0x581/0x66f [ 0.040993] [<c103a537>] ? sched_clock_cpu+0x136/0x142 [ 0.040993] [<c103a537>] ? sched_clock_cpu+0x136/0x142 [ 0.040993] [<c1039165>] ? ktime_get+0x13/0x2f [ 0.040993] [<c103a562>] ? sched_clock_idle_sleep_event+0xe/0x10 [ 0.040993] [<c102a86f>] ? __do_softirq+0x119/0x121 [ 0.040993] [<c116e282>] ? acpi_hw_low_level_read+0x3b/0x68 [ 0.040993] [<c116e34f>] ? acpi_hw_register_read+0xa0/0x112 [ 0.040993] [<c116e4f0>] ? acpi_get_register_unlocked+0x2c/0x48 [ 0.040993] [<c1161aa7>] ? acpi_os_release_lock+0x8/0xa [ 0.040993] [<c116e67b>] ? acpi_get_register+0x2d/0x34 [ 0.040993] [<c13d2ae0>] ? do_page_fault+0x0/0x66f [ 0.040993] [<c13d13da>] ? error_code+0x72/0x78 [ 0.040993] [<c16631e3>] ? kernel_init+0x0/0x148 [ 0.040993] [<c13b448b>] ? rest_init+0xf/0x57 [ 0.040993] [<c16637af>] ? start_kernel+0x2a2/0x2a7 [ 0.040993] [<c1663080>] ? __init_begin+0x80/0x88 [ 0.040993] Code: 89 4a 04 74 08 8d 41 0c e8 fa 04 d9 ff 5d c3 55 31 c9 89 e5 e8 e0 ff ff ff 5d c3 55 89 e5 57 56 89 c6 5 3 83 ec 04 8b 00 8b 7e 04 <8b> 50 10 89 55 f0 e8 7b cf 01 00 85 ff 74 23 8b 47 04 ba ec 42 [ 0.040993] EIP: [<c13b41dc>] klist_next+0x10/0x8d SS:ESP 0068:c165dd48
Quoted 5 lines
I know this is not much to fuzz about since it was artificially induced with the NULL pointer dereference, but what if such an error (a real one) made it into the kernel, it could scroll away the real oops. Anyway -- to reproduce, apply the patch and boot with panic=10 (1 also works). Thanks for the attention,
Reverting:
commit 70308923d317f2ad4973c30d90bb48ae38761317
Author: Greg Kroah-Hartman <[email protected]> Date: Wed Feb 13 22:30:39 2008 -0800
PCI: make no_pci_devices() use the pci_bus_type list
no_pci_devices() should use the driver core list of PCI devices, not our "separate" one.
fixes it.
Vegard
Signature · 4 lines
"The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
Quoted 132 lines · Vegard Nossum
Quoted 119 lines · Vegard Nossum
Hi,
With such a patch:
init/main.c +2
-0@@ -465,6 +465,8 @@ static void noinline __init_refok rest_init(void) {int pid; + *(char *)NULL =0 ; +kernel_thread (kernel_init,NULL ,CLONE_FS |CLONE_SIGHAND );numa_default_policy (); pid =kernel_thread (kthreadd,NULL ,CLONE_FS |CLONE_FILES );...I would expect a page fault and that's that. So
panic()is called, but it causes a new page fault somewhere. Here is the log:(this part is correct and expected)
[ 0.031003] BUG: unable to handle kernel NULL pointer dereference at 00000000 [ 0.033997] IP: [<c13b448b>] rest_init+0xf/0x57 [ 0.035997] *pde = 00000000 [ 0.037289] Oops: 0002 [#1] SMP [ 0.037994] last sysfs file: [ 0.037994] Modules linked in: [ 0.037994] [ 0.037994] Pid: 0, comm: swapper Not tainted (2.6.28-rc7 #181) 945P-A [ 0.037994] EIP: 0060:[<c13b448b>] EFLAGS: 00010246 CPU: 0 [ 0.037994] EIP is at rest_init+0xf/0x57 [ 0.037994] EAX: c16631e3 EBX: 00000040 ECX: 00000a00 EDX: 00000000 [ 0.037994] ESI: 00099800 EDI: c160a000 EBP: c165dfd0 ESP: c165dfd0 [ 0.037994] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 [ 0.037994] Process swapper (pid: 0, ti=c165c000 task=c156e334 task.ti=c165c000) [ 0.037994] Stack: [ 0.037994] c165dfe0 c16637af c1691768 00000000 c165dff8 c1663080 0175a000 00000000 [ 0.037994] c14ceaae 00020800 01b7e003 00000000 [ 0.037994] Call Trace: [ 0.037994] [<c16637af>] ? start_kernel+0x2a2/0x2a7 [ 0.037994] [<c1663080>] ? __init_begin+0x80/0x88 [ 0.037994] Code: 00 8b 43 04 8d 56 04 89 50 04 89 46 04 8d 43 04 89 46 08 89 53 04 fe 03 5b 5e 5d c3 55 b9 00 0a 00 00 8 9 e5 31 d2 b8 e3 31 66 c1 <c6> 05 00 00 00 00 00 e8 00 df c4 ff b9 00 06 00 00 31 d2 b8 af [ 0.037994] EIP: [<c13b448b>] rest_init+0xf/0x57 SS:ESP 0068:c165dfd0 [ 0.038004] ---[ end trace 4eaa2a86a8e2da22 ]--- [ 0.038998] Kernel panic - not syncing: Attempted to kill the idle task!And now the unexpected part:
[ 0.039999] Rebooting in 10 seconds..<1>BUG: unable to handle kernel NULL pointer dereference at 0000004c [ 0.040993] IP: [<c13b41dc>] klist_next+0x10/0x8d [ 0.040993] *pde = 00000000 [ 0.040993] Oops: 0000 [#2] SMP [ 0.040993] last sysfs file: [ 0.040993] Modules linked in: [ 0.040993] [ 0.040993] Pid: 0, comm: swapper Tainted: G D (2.6.28-rc7 #181) 945P-A [ 0.040993] EIP: 0060:[<c13b41dc>] EFLAGS: 00010286 CPU: 0 [ 0.040993] EIP is at klist_next+0x10/0x8d [ 0.040993] EAX: 0000003c EBX: c165dd60 ECX: 00000000 EDX: c165dd60 [ 0.040993] ESI: c165dd60 EDI: 00000000 EBP: c165dd58 ESP: c165dd48 [ 0.040993] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 [ 0.040993] Process swapper (pid: 0, ti=c165c000 task=c156e334 task.ti=c165c000) [ 0.040993] Stack: [ 0.040993] c1026a97 c165dd60 c165dd60 00000000 c165dd74 c11be196 0000003c 00000000 [ 0.040993] c13dbde0 00001078 00000100 c165dd84 c114ed26 c114e458 c13dbde0 c165dd9c [ 0.040993] c1152546 ffffffff c13dbde0 00002710 0000000b c165ddac c115259a ffffffff [ 0.040993] Call Trace: [ 0.040993] [<c1026a97>] ? release_console_sem+0x16c/0x199 [ 0.040993] [<c11be196>] ? bus_find_device+0x4e/0x6e [ 0.040993] [<c114ed26>] ? no_pci_devices+0x17/0x2d [ 0.040993] [<c114e458>] ? find_anything+0x0/0xa [ 0.040993] [<c1152546>] ? pci_get_subsys+0x15/0x5b [ 0.040993] [<c115259a>] ? pci_get_device+0xe/0x10 [ 0.040993] [<c1013342>] ? mach_reboot_fixups+0x27/0x3c [ 0.040993] [<c100fdfb>] ? native_machine_emergency_restart+0x3e/0xd7 [ 0.040993] [<c100fc60>] ? machine_emergency_restart+0x9/0xb [ 0.040993] [<c1032b4b>] ? emergency_restart+0x8/0xa [ 0.040993] [<c13cf607>] ? panic+0xb9/0xd6 [ 0.040993] [<c1028ee0>] ? do_exit+0x5b/0x740 [ 0.040993] [<c13cf633>] ? printk+0xf/0x11 [ 0.040993] [<c10263df>] ? print_oops_end_marker+0x1e/0x23 [ 0.040993] [<c13d1c1e>] ? oops_end+0x7f/0x87 [ 0.040993] [<c1005a08>] ? die+0x5b/0x63 [ 0.040993] [<c13d3061>] ? do_page_fault+0x581/0x66f [ 0.040993] [<c103a537>] ? sched_clock_cpu+0x136/0x142 [ 0.040993] [<c103a537>] ? sched_clock_cpu+0x136/0x142 [ 0.040993] [<c1039165>] ? ktime_get+0x13/0x2f [ 0.040993] [<c103a562>] ? sched_clock_idle_sleep_event+0xe/0x10 [ 0.040993] [<c102a86f>] ? __do_softirq+0x119/0x121 [ 0.040993] [<c116e282>] ? acpi_hw_low_level_read+0x3b/0x68 [ 0.040993] [<c116e34f>] ? acpi_hw_register_read+0xa0/0x112 [ 0.040993] [<c116e4f0>] ? acpi_get_register_unlocked+0x2c/0x48 [ 0.040993] [<c1161aa7>] ? acpi_os_release_lock+0x8/0xa [ 0.040993] [<c116e67b>] ? acpi_get_register+0x2d/0x34 [ 0.040993] [<c13d2ae0>] ? do_page_fault+0x0/0x66f [ 0.040993] [<c13d13da>] ? error_code+0x72/0x78 [ 0.040993] [<c16631e3>] ? kernel_init+0x0/0x148 [ 0.040993] [<c13b448b>] ? rest_init+0xf/0x57 [ 0.040993] [<c16637af>] ? start_kernel+0x2a2/0x2a7 [ 0.040993] [<c1663080>] ? __init_begin+0x80/0x88 [ 0.040993] Code: 89 4a 04 74 08 8d 41 0c e8 fa 04 d9 ff 5d c3 55 31 c9 89 e5 e8 e0 ff ff ff 5d c3 55 89 e5 57 56 89 c6 5 3 83 ec 04 8b 00 8b 7e 04 <8b> 50 10 89 55 f0 e8 7b cf 01 00 85 ff 74 23 8b 47 04 ba ec 42 [ 0.040993] EIP: [<c13b41dc>] klist_next+0x10/0x8d SS:ESP 0068:c165dd48Quoted 5 lines
I know this is not much to fuzz about since it was artificially induced with the NULL pointer dereference, but what if such an error (a real one) made it into the kernel, it could scroll away the real oops. Anyway -- to reproduce, apply the patch and boot with panic=10 (1 also works). Thanks for the attention,
Reverting:
commit
70308923d317f2ad4973c30d90bb48ae38761317Author: Greg Kroah-Hartman <[email protected]> Date: Wed Feb 13 22:30:39 2008 -0800
Quoted 8 lines
PCI: make
no_pci_devices()use thepci_bus_typelist
no_pci_devices()should use the driver core list of PCI devices, not our "separate" one.Signed-off-by Greg Kroah-Hartman <[email protected]>fixes it.
Fixes what? It might be quite difficult to revert that patch now, as the infrastructure is no longer in place to use a private pci device list, that code is long gone.
totally confused,
greg k-h
Hi Greg,
Quoted 3 lines · Vegard Nossum
Fixes what? It might be quite difficult to revert that patch now, as the infrastructure is no longer in place to use a private pci device list, that code is long gone.
Vegard forced one oops but got two! The first one is expected and but the second one shouldn't probably be there:
Quoted 57 lines
Quoted 57 lines
Quoted 57 lines
And now the unexpected part:
[ 0.039999] Rebooting in 10 seconds..<1>BUG: unable to handle kernel NULL pointer dereference at 0000004c [ 0.040993] IP: [<c13b41dc>] klist_next+0x10/0x8d [ 0.040993] *pde = 00000000 [ 0.040993] Oops: 0000 [#2] SMP [ 0.040993] last sysfs file: [ 0.040993] Modules linked in: [ 0.040993] [ 0.040993] Pid: 0, comm: swapper Tainted: G D (2.6.28-rc7 #181) 945P-A [ 0.040993] EIP: 0060:[<c13b41dc>] EFLAGS: 00010286 CPU: 0 [ 0.040993] EIP is at klist_next+0x10/0x8d [ 0.040993] EAX: 0000003c EBX: c165dd60 ECX: 00000000 EDX: c165dd60 [ 0.040993] ESI: c165dd60 EDI: 00000000 EBP: c165dd58 ESP: c165dd48 [ 0.040993] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 [ 0.040993] Process swapper (pid: 0, ti=c165c000 task=c156e334 task.ti=c165c000) [ 0.040993] Stack: [ 0.040993] c1026a97 c165dd60 c165dd60 00000000 c165dd74 c11be196 0000003c 00000000 [ 0.040993] c13dbde0 00001078 00000100 c165dd84 c114ed26 c114e458 c13dbde0 c165dd9c [ 0.040993] c1152546 ffffffff c13dbde0 00002710 0000000b c165ddac c115259a ffffffff [ 0.040993] Call Trace: [ 0.040993] [<c1026a97>] ? release_console_sem+0x16c/0x199 [ 0.040993] [<c11be196>] ? bus_find_device+0x4e/0x6e [ 0.040993] [<c114ed26>] ? no_pci_devices+0x17/0x2d [ 0.040993] [<c114e458>] ? find_anything+0x0/0xa [ 0.040993] [<c1152546>] ? pci_get_subsys+0x15/0x5b [ 0.040993] [<c115259a>] ? pci_get_device+0xe/0x10 [ 0.040993] [<c1013342>] ? mach_reboot_fixups+0x27/0x3c [ 0.040993] [<c100fdfb>] ? native_machine_emergency_restart+0x3e/0xd7 [ 0.040993] [<c100fc60>] ? machine_emergency_restart+0x9/0xb [ 0.040993] [<c1032b4b>] ? emergency_restart+0x8/0xa [ 0.040993] [<c13cf607>] ? panic+0xb9/0xd6 [ 0.040993] [<c1028ee0>] ? do_exit+0x5b/0x740 [ 0.040993] [<c13cf633>] ? printk+0xf/0x11 [ 0.040993] [<c10263df>] ? print_oops_end_marker+0x1e/0x23 [ 0.040993] [<c13d1c1e>] ? oops_end+0x7f/0x87 [ 0.040993] [<c1005a08>] ? die+0x5b/0x63 [ 0.040993] [<c13d3061>] ? do_page_fault+0x581/0x66f [ 0.040993] [<c103a537>] ? sched_clock_cpu+0x136/0x142 [ 0.040993] [<c103a537>] ? sched_clock_cpu+0x136/0x142 [ 0.040993] [<c1039165>] ? ktime_get+0x13/0x2f [ 0.040993] [<c103a562>] ? sched_clock_idle_sleep_event+0xe/0x10 [ 0.040993] [<c102a86f>] ? __do_softirq+0x119/0x121 [ 0.040993] [<c116e282>] ? acpi_hw_low_level_read+0x3b/0x68 [ 0.040993] [<c116e34f>] ? acpi_hw_register_read+0xa0/0x112 [ 0.040993] [<c116e4f0>] ? acpi_get_register_unlocked+0x2c/0x48 [ 0.040993] [<c1161aa7>] ? acpi_os_release_lock+0x8/0xa [ 0.040993] [<c116e67b>] ? acpi_get_register+0x2d/0x34 [ 0.040993] [<c13d2ae0>] ? do_page_fault+0x0/0x66f [ 0.040993] [<c13d13da>] ? error_code+0x72/0x78 [ 0.040993] [<c16631e3>] ? kernel_init+0x0/0x148
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
[ 0.040993] [<c13b448b>] ? rest_init+0xf/0x57 [ 0.040993] [<c16637af>] ? start_kernel+0x2a2/0x2a7 [ 0.040993] [<c1663080>] ? __init_begin+0x80/0x88 [ 0.040993] Code: 89 4a 04 74 08 8d 41 0c e8 fa 04 d9 ff 5d c3 55 31 c9 89 e5 e8 e0 ff ff ff 5d c3 55 89 e5 57 56 89 c6 5 3 83 ec 04 8b 00 8b 7e 04 <8b> 50 10 89 55 f0 e8 7b cf 01 00 85 ff 74 23 8b 47 04 ba ec 42 [ 0.040993] EIP: [<c13b41dc>] klist_next+0x10/0x8d SS:ESP 0068:c165dd48
Looks like the patch Vegard identified breaks something in the oops path?
Quoted 9 lines · Pekka Enberg
Hi Greg,
Quoted 3 lines · Vegard Nossum
Fixes what? It might be quite difficult to revert that patch now, as the infrastructure is no longer in place to use a private pci device list, that code is long gone.
Vegard forced one oops but got two! The first one is expected and but the second one shouldn't probably be there:
"Second" oopses are known to not be reliable, I wouldn't count it as a real problem unless it happens on its own.
Quoted 3 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
[ 0.040993] EIP: [<c13b41dc>] klist_next+0x10/0x8d SS:ESP 0068:c165dd48Looks like the patch Vegard identified breaks something in the oops path?
Very wierd, I also don't understand how reverting the specific patch would even make a buildable system.
thanks,
greg k-h
Quoted 8 lines · Greg KH
Quoted 7 lines · Pekka Enberg
Hi Greg,
Quoted 3 lines · Vegard Nossum
Fixes what? It might be quite difficult to revert that patch now, as the infrastructure is no longer in place to use a private pci device list, that code is long gone.
Quoted 5 lines
Quoted 2 lines
Vegard forced one oops but got two! The first one is expected and but the second one shouldn't probably be there:
"Second" oopses are known to not be reliable, I wouldn't count it as a real problem unless it happens on its own.
Yes, because usually it's a process that BUGed and was killed -- perhaps with locks held or in the middle of some transaction that will never complete. But this one happens in the panic code itself...
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
[ 0.040993] EIP: [<c13b41dc>] klist_next+0x10/0x8d SS:ESP 0068:c165dd48Looks like the patch Vegard identified breaks something in the oops path?
Very wierd, I also don't understand how reverting the specific patch would even make a buildable system.
It was an unclean revert, here's the relevant resultant hunk:
drivers/pci/probe.c +0 -0
@@@ -32,16 -29,55 +28,11 @@@ LIST_HEAD(pci_devices)*/ int no_pci_devices (void ) { -struct device *dev; -int no_devices; - - dev =bus_find_device (&pci_bus_type,NULL ,NULL , find_anything); - no_devices = (dev ==NULL ); -put_device (dev); -return no_devices; +return list_empty (&pci_devices); } +EXPORT_SYMBOL (no_pci_devices);
...and this builds.
The problem seems to be that pci_bus_type.p->klist_devices is NULL. Because:
Oops happens here:
struct klist_node *klist_next(struct klist_iter *i)
{
void (*put)(struct klist_node *) = i->i_klist->put;So either i == NULL or i->i_klist == NULL. But i->i_klist was just before set here:
void klist_iter_init_node(struct klist *k, struct klist_iter *i,
struct klist_node *n)
{
i->i_klist = k;
i->i_cur = n;
if (n)
kref_get(&n->n_ref);
}so the klist passed must have been NULL, it came from bus_find_device():
klist_iter_init_node(&bus->p->klist_devices, &i,
(start ? &start->knode_bus : NULL));...and indeed, printing bus->p here yields 00000000. This function was called from no_pci_devices(), so the bus variable was initialized from &pci_bus_type. So pci_bus_type.p == NULL.
This should be initialized in bus_register() called from pci_driver_init(). Aha, this never gets called because initcalls did not yet run.
A summary of the bug:
1. Sending panic=1 wants to reboot on panic.
2. If panic occurs before initcalls ran, pci_bus_type.p is not initialized.
3. mach_reboot_fixups() in x86 code calls pci_get_device()
4. New oops
Maybe mach_reboot_fixups() should check to see if PCI bus is initialized before calling pci_get_device(), since obviously it can be called before it has been initialized too.
The funny thing is that no_pci_devices() is what _used_ to guard against using pci_bus_type too early:
/*
* Some device drivers need know if pci is initiated.
* Basically, we think pci is not initiated when there
* is no device to be found on the pci_bus_type. */
int no_pci_devices(void)
...and now it uses pci_bus_type itself. That is what makes commit 70308923d317f2ad4973c30d90bb48ae38761317 wrong, because there might be other users of no_pci_devices() too, which would now almost certainly result in an Oops if the pci bus hasn't been initialized.
Please tell if any of the above is unclear, and I will try to explain more. Thanks,
Vegard
Signature · 4 lines
"The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
Quoted 106 lines · Vegard Nossum
Quoted 8 lines · Greg KH
Quoted 7 lines · Pekka Enberg
Hi Greg,
Quoted 3 lines · Vegard Nossum
Fixes what? It might be quite difficult to revert that patch now, as the infrastructure is no longer in place to use a private pci device list, that code is long gone.
Quoted 5 lines
Quoted 2 lines
Vegard forced one oops but got two! The first one is expected and but the second one shouldn't probably be there:
"Second" oopses are known to not be reliable, I wouldn't count it as a real problem unless it happens on its own.
Yes, because usually it's a process that BUGed and was killed -- perhaps with locks held or in the middle of some transaction that will never complete. But this one happens in the panic code itself...
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
[ 0.040993] EIP: [<c13b41dc>] klist_next+0x10/0x8d SS:ESP 0068:c165dd48Looks like the patch Vegard identified breaks something in the oops path?
Very wierd, I also don't understand how reverting the specific patch would even make a buildable system.
It was an unclean revert, here's the relevant resultant hunk:
drivers/pci/probe.c +2
-7@@@ -32,16 -29,55 +28,11 @@@ LIST_HEAD(pci_devices)*/ int no_pci_devices (void ) { -struct device *dev; -int no_devices; - - dev =bus_find_device (&pci_bus_type,NULL ,NULL , find_anything); - no_devices = (dev ==NULL ); -put_device (dev); -return no_devices; +return list_empty (&pci_devices); } +EXPORT_SYMBOL (no_pci_devices);...and this builds.
The problem seems to be that
pci_bus_type.p->klist_devicesis NULL. Because:Oops happens here:
struct
klist_node*klist_next(structklist_iter*i){ void (*put)(struct klist_node *) = i->i_klist->put;So either i == NULL or
i->i_klist== NULL. Buti->i_klistwas just before set here:void klist_iter_init_node(struct klist *k, struct
klist_iter*i,
structklist_node*n){ i->i_klist = k; i->i_cur = n; if (n) kref_get(&n->n_ref); }so the klist passed must have been NULL, it came from
bus_find_device():klist_iter_init_node(&bus->p->klist_devices, &i, (start ? &start->knode_bus : NULL));...and indeed, printing
bus->phere yields 00000000. This function was called fromno_pci_devices(), so the bus variable was initialized from&pci_bus_type. So pci_bus_type.p == NULL.This should be initialized in
bus_register()called frompci_driver_init(). Aha, this never gets called because initcalls did not yet run.A summary of the bug:
1. Sending panic=1 wants to reboot on panic.
2. If panic occurs before initcalls ran, pci_bus_type.p is not initialized.
3.mach_reboot_fixups()in x86 code callspci_get_device()
4. New oopsMaybe
mach_reboot_fixups()should check to see if PCI bus is initialized before callingpci_get_device(), since obviously it can be called before it has been initialized too.The funny thing is that
no_pci_devices()is what _used_ to guard against usingpci_bus_typetoo early:/*
* Some device drivers need know if pci is initiated.
* Basically, we think pci is not initiated when there
* is no device to be found on thepci_bus_type. */
intno_pci_devices(void)
Quoted 7 lines
...and now it uses
pci_bus_typeitself. That is what makes commit70308923d317f2ad4973c30d90bb48ae38761317wrong, because there might be other users ofno_pci_devices()too, which would now almost certainly result in an Oops if the pci bus hasn't been initialized.Please tell if any of the above is unclear, and I will try to explain more. Thanks,
No, that makes perfect sense.
Care to make a patch for no_pci_devices() to work properly in this kind of situation?
thanks,
greg k-h
PATCH 1/1 pci: fix no_pci_devices()
Quoted 2 lines · Greg KH
Care to make a patch for
no_pci_devices()to work properly in this kind of situation?
How does this look?
I have introduced a variable pci_is_initiated, which is set after the bus has been registered.
Should the variable be atomic? My assumption is that initcalls are synchronous and that the variable therefore cannot be accessed concurrently by two or more CPUs.
Vegard
Quoted 1 line
From
409132338c9a6122dc7da08d2764492dbf351f32Mon Sep 17 00:00:00 2001
From: Vegard Nossum <[email protected]>
Date: Sat, 20 Dec 2008 11:37:16 +0100
Subject: [PATCH] pci: fix no_pci_devices()
In short, no_pci_devices() should not use bus_find_device() before initcalls have run, because the pci bus structure has not been initialized yet.
Reference: http://lkml.org/lkml/2008/12/20/21
--- drivers/pci/pci-driver.c | 10 +++++++++- drivers/pci/probe.c | 3 +++ include/linux/pci.h | 1 + 3 files changed, 13 insertions(+), 1 deletions(-)
drivers/pci/pci-driver.c +9 -1
@@ -825,6 +825,8 @@ int pci_uevent(struct device *dev, struct kobj_uevent_env *env) }#endif +int pci_is_initiated__read_mostly ; +struct bus_type pci_bus_type = { .name ="pci" , .match = pci_bus_match,
@@ -838,7 +840,13 @@ struct bus_type pci_bus_type = {static int __init pci_driver_init (void ) { -return bus_register (&pci_bus_type); +int ret; + + ret =bus_register (&pci_bus_type); +if (ret ==0 ) + pci_is_initiated =1 ; + +return ret; }postcore_initcall (pci_driver_init);
drivers/pci/probe.c +3 -0
@@ -35,6 +35,9 @@ int no_pci_devices(void)struct device *dev;int no_devices; +if (!pci_is_initiated) +return 1 ; + dev =bus_find_device (&pci_bus_type,NULL ,NULL , find_anything); no_devices = (dev ==NULL );put_device (dev);
include/linux/pci.h +1 -0
@@ -484,6 +484,7 @@ extern struct bus_type pci_bus_type;* code, or pci core code. */ extern struct list_head pci_root_buses;/* list of all known PCI buses */ /* Some device drivers need know if pci is initiated */ +extern int pci_is_initiated;extern int no_pci_devices (void );void pcibios_fixup_bus (struct pci_bus *);
Signature · 1 line
1.5.6.5
PATCH 1/1 pci: fix no_pci_devices() #2
Quoted 8 lines · Vegard Nossum
Quoted 2 lines · Greg KH
Care to make a patch for
no_pci_devices()to work properly in this kind of situation?How does this look?
I have introduced a variable
pci_is_initiated, which is set after the bus has been registered.
This patch is simpler and also works for me. But I am not too fond of it either...
Vegard
Quoted 1 line
From
1f047c86fc7a831d85174452da92344a3582a158Mon Sep 17 00:00:00 2001
From: Vegard Nossum <[email protected]>
Date: Sat, 20 Dec 2008 12:08:18 +0100
Subject: [PATCH] pci: fix no_pci_devices() #2
In short, no_pci_devices() should not use bus_find_device() before initcalls have run, because the pci bus structure has not been initialized yet.
Reference: http://lkml.org/lkml/2008/12/20/21
--- drivers/base/bus.c | 1 + drivers/pci/probe.c | 3 +++ 2 files changed, 4 insertions(+), 0 deletions(-)
drivers/base/bus.c +1 -0
@@ -931,6 +931,7 @@ bus_devices_fail: bus_uevent_fail:kset_unregister (&bus->p->subsys);kfree (bus->p); + bus->p =NULL ; out:return retval; }
drivers/pci/probe.c +3 -0
@@ -35,6 +35,9 @@ int no_pci_devices(void)struct device *dev;int no_devices; +if (!pci_bus_type.p) +return 1 ; + dev =bus_find_device (&pci_bus_type,NULL ,NULL , find_anything); no_devices = (dev ==NULL );put_device (dev);
Signature · 1 line
1.5.6.5
Re: [PATCH] pci: fix no_pci_devices() #2
Quoted 41 lines · Vegard Nossum
Quoted 8 lines · Vegard Nossum
Quoted 2 lines · Greg KH
Care to make a patch for
no_pci_devices()to work properly in this kind of situation?How does this look?
I have introduced a variable
pci_is_initiated, which is set after the bus has been registered.This patch is simpler and also works for me. But I am not too fond of it either...
Vegard
Quoted 1 line
From
1f047c86fc7a831d85174452da92344a3582a158Mon Sep 17 00:00:00 2001From: Vegard Nossum <[email protected]>
Date: Sat, 20 Dec 2008 12:08:18 +0100
Subject: [PATCH] pci: fixno_pci_devices()#2In short,
no_pci_devices()should not usebus_find_device()before initcalls have run, because the pci bus structure has not been initialized yet.Reference: http://lkml.org/lkml/2008/12/20/21
Cc Greg KH <[email protected]>Cc Pekka Enberg <[email protected]>Signed-off-by Vegard Nossum <[email protected]>--- drivers/base/bus.c | 1 + drivers/pci/probe.c | 3 +++ 2 files changed, 4 insertions(+), 0 deletions(-)drivers/base/bus.c +0
-0
drivers/base/bus.c bus_devices_fail: · line 936
Quoted 6 lines
drivers/base/bus.c +1
-0@@ -931,6 +931,7 @@ bus_devices_fail: bus_uevent_fail:kset_unregister (&bus->p->subsys);kfree (bus->p); + bus->p =NULL ; out:return retval;
I like this portion anyway, care to break this out into a separate patch and send it to me?
The first one also looks good to me, Jesse, feel free to add my:
thanks,
greg k-h
Re: [PATCH] pci: fix no_pci_devices() #2
In reply to Vegard NossumQuoted 1 line · Vegard Nossum
On Sat, Dec 20, 2008 at 11:56 AM, Vegard Nossum <[email protected]>
Quoted 52 lines · wrote:
Quoted 8 lines · Greg KH
Quoted 2 lines
Care to make a patch for
no_pci_devices()to work properly in this kind of situation?How does this look?
I have introduced a variable
pci_is_initiated, which is set after the bus has been registered.This patch is simpler and also works for me. But I am not too fond of it either...
Vegard
From
1f047c86fc7a831d85174452da92344a3582a158Mon Sep 17 00:00:00 2001
From: Vegard Nossum <[email protected]>
Date: Sat, 20 Dec 2008 12:08:18 +0100
Subject: [PATCH] pci: fixno_pci_devices()#2In short,
no_pci_devices()should not usebus_find_device()before initcalls have run, because the pci bus structure has not been initialized yet.Reference: http://lkml.org/lkml/2008/12/20/21
Cc Greg KH <[email protected]>Cc Pekka Enberg <[email protected]>Signed-off-by Vegard Nossum <[email protected]>--- drivers/base/bus.c | 1 + drivers/pci/probe.c | 3 +++ 2 files changed, 4 insertions(+), 0 deletions(-)drivers/base/bus.c +1
-0@@ -931,6 +931,7 @@ bus_devices_fail: bus_uevent_fail:kset_unregister (&bus->p->subsys);kfree (bus->p); + bus->p =NULL ; out:return retval; }drivers/pci/probe.c +0
-0
drivers/pci/probe.c int no_pci_devices(void) · line 43
Quoted 9 lines
drivers/pci/probe.c +3
-0@@ -35,6 +35,9 @@ int no_pci_devices(void)struct device *dev;int no_devices; +if (!pci_bus_type.p) +return 1 ; + dev =bus_find_device (&pci_bus_type,NULL ,NULL , find_anything); no_devices = (dev ==NULL );put_device (dev);
Assuming Greg already took the generic part, can you resend the PCI part to the [email protected] list for review just in case anyone has a better idea of how to do it?
Thanks,
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH] pci: fix no_pci_devices() #2
Quoted 58 lines · Jesse Barnes
Quoted 1 line · Vegard Nossum
On Sat, Dec 20, 2008 at 11:56 AM, Vegard Nossum <[email protected]>
Quoted 55 lines · wrote:
Quoted 8 lines · Greg KH
Quoted 2 lines
Care to make a patch for
no_pci_devices()to work properly in this kind of situation?How does this look?
I have introduced a variable
pci_is_initiated, which is set after the bus has been registered.This patch is simpler and also works for me. But I am not too fond of it either...
Vegard
From
1f047c86fc7a831d85174452da92344a3582a158Mon Sep 17 00:00:00 2001
From: Vegard Nossum <[email protected]>
Date: Sat, 20 Dec 2008 12:08:18 +0100
Subject: [PATCH] pci: fixno_pci_devices()#2In short,
no_pci_devices()should not usebus_find_device()before initcalls have run, because the pci bus structure has not been initialized yet.Reference: http://lkml.org/lkml/2008/12/20/21
Cc Greg KH <[email protected]>Cc Pekka Enberg <[email protected]>Signed-off-by Vegard Nossum <[email protected]>--- drivers/base/bus.c | 1 + drivers/pci/probe.c | 3 +++ 2 files changed, 4 insertions(+), 0 deletions(-)drivers/base/bus.c +1
-0@@ -931,6 +931,7 @@ bus_devices_fail: bus_uevent_fail:kset_unregister (&bus->p->subsys);kfree (bus->p); + bus->p =NULL ; out:return retval; }drivers/pci/probe.c +0
-0@@ -35,6 +35,9 @@ int no_pci_devices(void)struct device *dev;int no_devices;
Quoted 10 lines
drivers/pci/probe.c int no_pci_devices(void) · line 43
Quoted 6 lines
drivers/pci/probe.c +3
-0+if (!pci_bus_type.p) +return 1 ; + dev =bus_find_device (&pci_bus_type,NULL ,NULL , find_anything); no_devices = (dev ==NULL );put_device (dev);Assuming Greg already took the generic part, can you resend the PCI part to the [email protected] list for review just in case anyone has a better idea of how to do it?
Did I take the generic part? I can't remember...
thanks,
greg k-h
Re: [PATCH] pci: fix no_pci_devices() #2
Quoted 72 lines · Greg KH
Quoted 60 lines · Jesse Barnes
Quoted 2 lines · Vegard Nossum
On Sat, Dec 20, 2008 at 11:56 AM, Vegard Nossum
<[email protected]>Quoted 55 lines · wrote:
Quoted 8 lines · Greg KH
Quoted 2 lines
Care to make a patch for
no_pci_devices()to work properly in this kind of situation?How does this look?
I have introduced a variable
pci_is_initiated, which is set after the bus has been registered.This patch is simpler and also works for me. But I am not too fond of it either...
Vegard
From
1f047c86fc7a831d85174452da92344a3582a158Mon Sep 17 00:00:00 2001
From: Vegard Nossum <[email protected]>
Date: Sat, 20 Dec 2008 12:08:18 +0100
Subject: [PATCH] pci: fixno_pci_devices()#2In short,
no_pci_devices()should not usebus_find_device()before initcalls have run, because the pci bus structure has not been initialized yet.Reference: http://lkml.org/lkml/2008/12/20/21
Cc Greg KH <[email protected]>Cc Pekka Enberg <[email protected]>Signed-off-by Vegard Nossum <[email protected]>--- drivers/base/bus.c | 1 + drivers/pci/probe.c | 3 +++ 2 files changed, 4 insertions(+), 0 deletions(-)drivers/base/bus.c +1
-0@@ -931,6 +931,7 @@ bus_devices_fail: bus_uevent_fail:kset_unregister (&bus->p->subsys);kfree (bus->p); + bus->p =NULL ; out:return retval; }drivers/pci/probe.c +0
-0@@ -35,6 +35,9 @@ int no_pci_devices(void)struct device *dev;int no_devices;Quoted 10 lines
drivers/pci/probe.c int no_pci_devices(void) · line 43
Quoted 6 lines
drivers/pci/probe.c +3
-0+if (!pci_bus_type.p) +return 1 ; + dev =bus_find_device (&pci_bus_type,NULL ,NULL , find_anything); no_devices = (dev ==NULL );put_device (dev);Assuming Greg already took the generic part, can you resend the PCI part to the [email protected] list for review just in case anyone has a better idea of how to do it?
Quoted 1 line
Did I take the generic part? I can't remember...
Doesn't look like it. Vergard can you send out an updated patch set?
Thanks,
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
Re: [PATCH] pci: fix no_pci_devices() #2
Quoted 7 lines · Jesse Barnes
Quoted 5 lines
Quoted 3 lines
Assuming Greg already took the generic part, can you resend the PCI part to the [email protected] list for review just in case anyone has a better idea of how to do it?
Did I take the generic part? I can't remember...
Doesn't look like it. Vergard can you send out an updated patch set?
Actually, my patch is still just a hack, since pci_bus_type.p is still set before the pci_bus_type is really usable. So if the kernel crashes (or, in general, no_pci_devices() is called) at some point between the pci_bus_type.p = <something> and pci_bus_type.p = NULL (which I inserted), we will still see the same type of fault.
So I would prefer to solve this in a different way, like a dedicated flag which is only set after we know that pci_bus_type initialisation succeeded. I think that was the approach of my first patch? I don't remember. In any case, such a patch could not be split in generic/pci parts, I think. Also, should we anticipate concurrent access to pci_bus_type.p or such a dedicated "no_pci_devices" flag?
Here is the first patch, but I wonder if it should be turned into atomic_t instead: http://lkml.org/lkml/2008/12/20/33
Vegard
Signature · 4 lines
"The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
Re: [PATCH] pci: fix no_pci_devices() #2
Quoted 1 line · Vegard Nossum
On Fri, Jan 16, 2009 at 7:41 PM, Jesse Barnes <[email protected]>
Quoted 21 lines · wrote:
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Assuming Greg already took the generic part, can you resend the PCI part to the [email protected] list for review just in case anyone has a better idea of how to do it?
Did I take the generic part? I can't remember...
Doesn't look like it. Vergard can you send out an updated patch set?
Actually, my patch is still just a hack, since pci_bus_type.p is still set before the
pci_bus_typeis really usable. So if the kernel crashes (or, in general,no_pci_devices()is called) at some point between the pci_bus_type.p = <something> and pci_bus_type.p = NULL (which I inserted), we will still see the same type of fault.So I would prefer to solve this in a different way, like a dedicated flag which is only set after we know that
pci_bus_typeinitialisation succeeded. I think that was the approach of my first patch? I don't remember. In any case, such a patch could not be split in generic/pci parts, I think. Also, should we anticipate concurrent access to pci_bus_type.p or such a dedicated "no_pci_devices" flag?
Quoted 2 lines
Here is the first patch, but I wonder if it should be turned into
atomic_tinstead: http://lkml.org/lkml/2008/12/20/33
It seems like you could do this with a driver core call? Isn't there a way to check whether a given bus type is registered? If so, we could just use that from no_pci_devices instead of a new flag.
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center