Re: [PATCH] 2.6.24-rc2: fix oops in ACPI battery removal
Andrey, Fix already exists. It already went up to Len's tree. Regards, Alex. Andrey Borzenkov wrote: Alexey, this fixes my patch that went into -rc2. Sorry for Oops. Subject: [PATCH] 2.6.24-rc2: do not unregister power_supply in sysfs ->show method From: Andrey Borzenkov <[EMAIL PROTECTED]> Currently unplugging battery oopses with call stack: [ 2245.375393] [] show_trace_log_lvl+0x1a/0x30 [ 2245.375570] [] show_stack_log_lvl+0xa9/0xd0 [ 2245.375732] [] show_registers+0xc6/0x1c0 [ 2245.375884] [] die+0xf1/0x200 [ 2245.376014] [] do_page_fault+0x198/0x690 [ 2245.376207] [] error_code+0x6a/0x70 [ 2245.376366] [] device_del+0x20/0x280 [ 2245.376530] [] device_unregister+0xb/0x20 [ 2245.376686] [] power_supply_unregister+0x1a/0x20 [power_supply] [ 2245.376911] [] sysfs_remove_battery+0x1f/0x22 [battery] [ 2245.382969] [] acpi_battery_update+0x42/0x293 [battery] [ 2245.388862] [] acpi_battery_get_property+0x34/0x16e [battery] [ 2245.394770] [] power_supply_show_property+0x38/0x130 [power_supply] [ 2245.400733] [] power_supply_uevent+0x111/0x1c0 [power_supply] [ 2245.406745] [] dev_uevent+0x99/0x100 [ 2245.412744] [] kobject_uevent_env+0x17f/0x3c0 [ 2245.418768] [] kobject_uevent+0xa/0x10 [ 2245.424732] [] device_del+0x1a3/0x280 [ 2245.430683] [] device_unregister+0xb/0x20 [ 2245.436632] [] power_supply_unregister+0x1a/0x20 [power_supply] [ 2245.442670] [] sysfs_remove_battery+0x1f/0x22 [battery] [ 2245.448790] [] acpi_battery_update+0x42/0x293 [battery] [ 2245.454874] [] acpi_battery_notify+0x1e/0x69 [battery] [ 2245.460957] [] acpi_ev_notify_dispatch+0x4c/0x57 [ 2245.467077] [] acpi_os_execute_notify+0x24/0x2f [ 2245.473167] [] run_workqueue+0x161/0x1e0 [ 2245.479289] [] worker_thread+0x8b/0xf0 [ 2245.485279] [] kthread+0x42/0x70 [ 2245.491068] [] kernel_thread_helper+0x7/0x14 Do not try to (un-)register sysfs in acpi_battery_update - instead, do it only on battery add or in notification handler. On the way fix battery removal (reset device to NULL) and use proper interface for sending change event (power_supply_changed) instead of doing it directly. Signed-off-by: Andrey Borzenkov <[EMAIL PROTECTED]> --- drivers/acpi/battery.c | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 1905b88..2810a9b 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -457,6 +457,7 @@ static void sysfs_remove_battery(struct acpi_battery *battery) return; device_remove_file(battery->bat.dev, _attr); power_supply_unregister(>bat); + battery->bat.dev = NULL; } static int acpi_battery_update(struct acpi_battery *battery) @@ -464,12 +465,6 @@ static int acpi_battery_update(struct acpi_battery *battery) int result = acpi_battery_get_status(battery); if (result) return result; - if (!acpi_battery_present(battery)) { - sysfs_remove_battery(battery); - return 0; - } - if (!battery->bat.dev) - sysfs_add_battery(battery); return acpi_battery_get_state(battery); } @@ -758,14 +753,17 @@ static void acpi_battery_notify(acpi_handle handle, u32 event, void *data) return; device = battery->device; acpi_battery_update(battery); + if (!acpi_battery_present(battery)) + sysfs_remove_battery(battery); + else if (!battery->bat.dev) + sysfs_add_battery(battery); + else + power_supply_changed(>bat); acpi_bus_generate_proc_event(device, event, acpi_battery_present(battery)); acpi_bus_generate_netlink_event(device->pnp.device_class, device->dev.bus_id, event, acpi_battery_present(battery)); - /* acpi_batter_update could remove power_supply object */ - if (battery->bat.dev) - kobject_uevent(>bat.dev->kobj, KOBJ_CHANGE); } static int acpi_battery_add(struct acpi_device *device) @@ -784,6 +782,8 @@ static int acpi_battery_add(struct acpi_device *device) acpi_driver_data(device) = battery; mutex_init(>lock); acpi_battery_update(battery); + if (acpi_battery_present(battery)) + sysfs_add_battery(battery); #ifdef CONFIG_ACPI_PROCFS result = acpi_battery_add_fs(device); if (result) - 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/
Re: [PATCH] 2.6.24-rc2: fix oops in ACPI battery removal
Andrey, Fix already exists. It already went up to Len's tree. Regards, Alex. Andrey Borzenkov wrote: Alexey, this fixes my patch that went into -rc2. Sorry for Oops. Subject: [PATCH] 2.6.24-rc2: do not unregister power_supply in sysfs -show method From: Andrey Borzenkov [EMAIL PROTECTED] Currently unplugging battery oopses with call stack: [ 2245.375393] [c010488a] show_trace_log_lvl+0x1a/0x30 [ 2245.375570] [c0104949] show_stack_log_lvl+0xa9/0xd0 [ 2245.375732] [c0104a36] show_registers+0xc6/0x1c0 [ 2245.375884] [c0104c21] die+0xf1/0x200 [ 2245.376014] [c02d2a58] do_page_fault+0x198/0x690 [ 2245.376207] [c02d1522] error_code+0x6a/0x70 [ 2245.376366] [c0243c20] device_del+0x20/0x280 [ 2245.376530] [c0243e8b] device_unregister+0xb/0x20 [ 2245.376686] [dfd7401a] power_supply_unregister+0x1a/0x20 [power_supply] [ 2245.376911] [dfd5f035] sysfs_remove_battery+0x1f/0x22 [battery] [ 2245.382969] [dfd5f1e6] acpi_battery_update+0x42/0x293 [battery] [ 2245.388862] [dfd5f5f5] acpi_battery_get_property+0x34/0x16e [battery] [ 2245.394770] [dfd74368] power_supply_show_property+0x38/0x130 [power_supply] [ 2245.400733] [dfd746f1] power_supply_uevent+0x111/0x1c0 [power_supply] [ 2245.406745] [c0244559] dev_uevent+0x99/0x100 [ 2245.412744] [c01d37ff] kobject_uevent_env+0x17f/0x3c0 [ 2245.418768] [c01d3a4a] kobject_uevent+0xa/0x10 [ 2245.424732] [c0243da3] device_del+0x1a3/0x280 [ 2245.430683] [c0243e8b] device_unregister+0xb/0x20 [ 2245.436632] [dfd7401a] power_supply_unregister+0x1a/0x20 [power_supply] [ 2245.442670] [dfd5f035] sysfs_remove_battery+0x1f/0x22 [battery] [ 2245.448790] [dfd5f1e6] acpi_battery_update+0x42/0x293 [battery] [ 2245.454874] [dfd5f7ad] acpi_battery_notify+0x1e/0x69 [battery] [ 2245.460957] [c02086cd] acpi_ev_notify_dispatch+0x4c/0x57 [ 2245.467077] [c0200d54] acpi_os_execute_notify+0x24/0x2f [ 2245.473167] [c012a431] run_workqueue+0x161/0x1e0 [ 2245.479289] [c012ae5b] worker_thread+0x8b/0xf0 [ 2245.485279] [c012e0b2] kthread+0x42/0x70 [ 2245.491068] [c01044e3] kernel_thread_helper+0x7/0x14 Do not try to (un-)register sysfs in acpi_battery_update - instead, do it only on battery add or in notification handler. On the way fix battery removal (reset device to NULL) and use proper interface for sending change event (power_supply_changed) instead of doing it directly. Signed-off-by: Andrey Borzenkov [EMAIL PROTECTED] --- drivers/acpi/battery.c | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 1905b88..2810a9b 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -457,6 +457,7 @@ static void sysfs_remove_battery(struct acpi_battery *battery) return; device_remove_file(battery-bat.dev, alarm_attr); power_supply_unregister(battery-bat); + battery-bat.dev = NULL; } static int acpi_battery_update(struct acpi_battery *battery) @@ -464,12 +465,6 @@ static int acpi_battery_update(struct acpi_battery *battery) int result = acpi_battery_get_status(battery); if (result) return result; - if (!acpi_battery_present(battery)) { - sysfs_remove_battery(battery); - return 0; - } - if (!battery-bat.dev) - sysfs_add_battery(battery); return acpi_battery_get_state(battery); } @@ -758,14 +753,17 @@ static void acpi_battery_notify(acpi_handle handle, u32 event, void *data) return; device = battery-device; acpi_battery_update(battery); + if (!acpi_battery_present(battery)) + sysfs_remove_battery(battery); + else if (!battery-bat.dev) + sysfs_add_battery(battery); + else + power_supply_changed(battery-bat); acpi_bus_generate_proc_event(device, event, acpi_battery_present(battery)); acpi_bus_generate_netlink_event(device-pnp.device_class, device-dev.bus_id, event, acpi_battery_present(battery)); - /* acpi_batter_update could remove power_supply object */ - if (battery-bat.dev) - kobject_uevent(battery-bat.dev-kobj, KOBJ_CHANGE); } static int acpi_battery_add(struct acpi_device *device) @@ -784,6 +782,8 @@ static int acpi_battery_add(struct acpi_device *device) acpi_driver_data(device) = battery; mutex_init(battery-lock); acpi_battery_update(battery); + if (acpi_battery_present(battery)) + sysfs_add_battery(battery); #ifdef CONFIG_ACPI_PROCFS result = acpi_battery_add_fs(device); if (result) - 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/