Re: [PATCH] 2.6.24-rc2: fix oops in ACPI battery removal

2007-11-10 Thread Alexey Starikovskiy

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

2007-11-10 Thread Alexey Starikovskiy

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/