Re: 2.6.24-rc1: OOPS at acpi_battery_update

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Andrew,

I can not contact with Len for several days, while the oops on battery seems quite important.
It also seem to behave well in -mm tree (as part of Len's acpi-test).
Will you send this patch to Linus without approval from Len or should I?

Thanks,
Alex.

Andrew Morton wrote:
On Fri, 09 Nov 2007 12:36:43 +0300 Alexey Starikovskiy <[email protected]> wrote:

Andrew Morton wrote:
A> On Thu, 08 Nov 2007 19:35:23 +0300 Alexey Starikovskiy <[email protected]> wrote:
[remove_cycle_at_battery_removal.patch  text/x-patch (1.7KB)]
ACPI: Battery: remove cycle from battery removal.

From: Alexey Starikovskiy <[email protected]>

get_property() should not call battery_update() on absent battery to
avoid cycle and oops.

Signed-off-by: Alexey Starikovskiy <[email protected]>
Tested-by: Rolf Eike Beer <[email protected]>
A patch similar to this one but with an identical changelog was merged into
Len's tree on November 2.

If it had been promptly merged into mainline then quite a lot of people's
time would not have been wasted.

Andrew,
should I send such patches directly to you/Linus?


Well if Len doesn't object and you're confident in the changes, why not? Any time we leave bugs unfixed we drive away testers and that impacts all
parts of the kernel.


ACPI: Battery: remove cycle from battery removal.

From: Alexey Starikovskiy <[email protected]>

get_property() should not call battery_update() on absent battery to
avoid cycle and oops.

Signed-off-by: Alexey Starikovskiy <[email protected]>
Tested-by: Rolf Eike Beer <[email protected]>
---

 drivers/acpi/battery.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c2ce0ad..192c244 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -132,7 +132,7 @@ static int acpi_battery_technology(struct acpi_battery *battery)
 	return POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
 }
 
-static int acpi_battery_update(struct acpi_battery *battery);
+static int acpi_battery_get_state(struct acpi_battery *battery);
 
 static int acpi_battery_get_property(struct power_supply *psy,
 				     enum power_supply_property psp,
@@ -140,10 +140,11 @@ static int acpi_battery_get_property(struct power_supply *psy,
 {
 	struct acpi_battery *battery = to_acpi_battery(psy);
 
-	if ((!acpi_battery_present(battery)) &&
-	     psp != POWER_SUPPLY_PROP_PRESENT)
+	if (acpi_battery_present(battery)) {
+		/* run battery update only if it is present */
+		acpi_battery_get_state(battery);
+	} else if (psp != POWER_SUPPLY_PROP_PRESENT)
 		return -ENODEV;
-	acpi_battery_update(battery);
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:
 		if (battery->state & 0x01)
@@ -457,6 +458,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)

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux