From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754109AbXKJSrw (ORCPT ); Sat, 10 Nov 2007 13:47:52 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752856AbXKJSro (ORCPT ); Sat, 10 Nov 2007 13:47:44 -0500 Received: from charybdis-ext.suse.de ([195.135.221.2]:59484 "EHLO emea5-mh.id5.novell.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752745AbXKJSrn (ORCPT ); Sat, 10 Nov 2007 13:47:43 -0500 Message-ID: <4735FCCD.3070302@suse.de> Date: Sat, 10 Nov 2007 21:47:41 +0300 From: Alexey Starikovskiy User-Agent: Thunderbird 2.0.0.6 (X11/20071022) MIME-Version: 1.0 To: Andrey Borzenkov CC: linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] 2.6.24-rc2: fix oops in ACPI battery removal References: <200711102140.49304.arvidjaar@mail.ru> In-Reply-To: <200711102140.49304.arvidjaar@mail.ru> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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 > > --- > > 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)