Here is a patch that should fix the battery hangs on RELENG_6. It was
tested to work fine, although I need testing from an affected user to
verify it fixes the problem. It was committed to HEAD and will be MFCed
if it fixes the problem.
I'm a bit disappointed that no one reported this problem in the 2 weeks
it was present in 7-current. If you have the time to run -current on at
least one partition of your laptop, that would assist me greatly.
Thanks,
-Nate
-------- Original Message --------
Subject: cvs commit: src/sys/dev/acpica acpi_cmbat.c
Date: Wed, 23 Nov 2005 00:58:05 +0000 (GMT)
From: Nate Lawson <njl@FreeBSD.org>
To: njl@FreeBSD.ORG
njl 2005-11-23 00:57:51 UTC
FreeBSD src repository
Modified files:
sys/dev/acpica acpi_cmbat.c
Log:
Try to fix problems with periodic hangs by never directly calling _BIF.
Instead, re-evaluate _BIF only when we get a notify and use the cached
results. We also still evaluate _BIF once on boot. Also, optimize the
init loop a little by only querying for a particular info if it's not
valid.
MFC after: 2 days
Revision Changes Path
1.42 +34 -22 src/sys/dev/acpica/acpi_cmbat.c
Index: src/sys/dev/acpica/acpi_cmbat.c
diff -u src/sys/dev/acpica/acpi_cmbat.c:1.41
src/sys/dev/acpica/acpi_cmbat.c:1.42
--- src/sys/dev/acpica/acpi_cmbat.c:1.41 Sun Sep 11 18:39:01 2005
+++ src/sys/dev/acpica/acpi_cmbat.c Wed Nov 23 00:57:51 2005
@@ -66,7 +66,6 @@
struct acpi_bif bif;
struct acpi_bst bst;
- struct timespec bif_lastupdated;
struct timespec bst_lastupdated;
};
@@ -80,8 +79,8 @@
void *context);
static int acpi_cmbat_info_expired(struct timespec *lastupdated);
static void acpi_cmbat_info_updated(struct timespec *lastupdated);
-static void acpi_cmbat_get_bst(device_t dev);
-static void acpi_cmbat_get_bif(device_t dev);
+static void acpi_cmbat_get_bst(void *arg);
+static void acpi_cmbat_get_bif(void *arg);
static int acpi_cmbat_bst(device_t dev, struct acpi_bst *bstp);
static int acpi_cmbat_bif(device_t dev, struct acpi_bif *bifp);
static void acpi_cmbat_init_battery(void *arg);
@@ -134,7 +133,6 @@
handle = acpi_get_handle(dev);
sc->dev = dev;
- timespecclear(&sc->bif_lastupdated);
timespecclear(&sc->bst_lastupdated);
error = acpi_battery_register(dev);
@@ -180,20 +178,22 @@
dev = (device_t)context;
sc = device_get_softc(dev);
- /*
- * Clear the appropriate last updated time. The next call to retrieve
- * the battery status will get the new value for us. We don't need to
- * acquire a lock since we are only clearing the time stamp and since
- * calling _BST/_BIF can trigger a notify, we could deadlock also.
- */
switch (notify) {
case ACPI_NOTIFY_DEVICE_CHECK:
case ACPI_BATTERY_BST_CHANGE:
+ /*
+ * Clear the last updated time. The next call to retrieve the
+ * battery status will get the new value for us.
+ */
timespecclear(&sc->bst_lastupdated);
break;
case ACPI_NOTIFY_BUS_CHECK:
case ACPI_BATTERY_BIF_CHANGE:
- timespecclear(&sc->bif_lastupdated);
+ /*
+ * Queue a callback to get the current battery info from thread
+ * context. It's not safe to block in a notify handler.
+ */
+ AcpiOsQueueForExecution(OSD_PRIORITY_LO, acpi_cmbat_get_bif, dev);
break;
}
@@ -229,16 +229,18 @@
}
static void
-acpi_cmbat_get_bst(device_t dev)
+acpi_cmbat_get_bst(void *arg)
{
struct acpi_cmbat_softc *sc;
ACPI_STATUS as;
ACPI_OBJECT *res;
ACPI_HANDLE h;
ACPI_BUFFER bst_buffer;
+ device_t dev;
ACPI_SERIAL_ASSERT(cmbat);
+ dev = arg;
sc = device_get_softc(dev);
h = acpi_get_handle(dev);
bst_buffer.Pointer = NULL;
@@ -287,24 +289,23 @@
}
static void
-acpi_cmbat_get_bif(device_t dev)
+acpi_cmbat_get_bif(void *arg)
{
struct acpi_cmbat_softc *sc;
ACPI_STATUS as;
ACPI_OBJECT *res;
ACPI_HANDLE h;
ACPI_BUFFER bif_buffer;
+ device_t dev;
ACPI_SERIAL_ASSERT(cmbat);
+ dev = arg;
sc = device_get_softc(dev);
h = acpi_get_handle(dev);
bif_buffer.Pointer = NULL;
bif_buffer.Length = ACPI_ALLOCATE_BUFFER;
- if (!acpi_cmbat_info_expired(&sc->bif_lastupdated))
- goto end;
-
as = AcpiEvaluateObject(h, "_BIF", NULL, &bif_buffer);
if (ACPI_FAILURE(as)) {
ACPI_VPRINT(dev, acpi_device_get_parent_softc(dev),
@@ -346,7 +347,6 @@
goto end;
if (acpi_PkgStr(res, 12, sc->bif.oeminfo, ACPI_CMBAT_MAXSTRLEN) != 0)
goto end;
- acpi_cmbat_info_updated(&sc->bif_lastupdated);
end:
if (bif_buffer.Pointer != NULL)
@@ -360,8 +360,13 @@
sc = device_get_softc(dev);
+ /*
+ * Just copy the data. The only value that should change is the
+ * last-full capacity, so we only update when we get a notify that says
+ * the info has changed. Many systems apparently take a long time to
+ * process a _BIF call so we avoid it if possible.
+ */
ACPI_SERIAL_BEGIN(cmbat);
- acpi_cmbat_get_bif(dev);
bifp->units = sc->bif.units;
bifp->dcap = sc->bif.dcap;
bifp->lfcap = sc->bif.lfcap;
@@ -422,11 +427,18 @@
if (!acpi_BatteryIsPresent(dev))
continue;
+ /*
+ * Only query the battery if this is the first try or the specific
+ * type of info is still invalid.
+ */
ACPI_SERIAL_BEGIN(cmbat);
- timespecclear(&sc->bst_lastupdated);
- timespecclear(&sc->bif_lastupdated);
- acpi_cmbat_get_bst(dev);
- acpi_cmbat_get_bif(dev);
+ if (retry == 0 || !acpi_battery_bst_valid(&sc->bst)) {
+ timespecclear(&sc->bst_lastupdated);
+ acpi_cmbat_get_bst(dev);
+ }
+ if (retry == 0 || !acpi_battery_bif_valid(&sc->bif))
+ acpi_cmbat_get_bif(dev);
+
valid = acpi_battery_bst_valid(&sc->bst) &&
acpi_battery_bif_valid(&sc->bif);
ACPI_SERIAL_END(cmbat);
--
Nate