Performance problem since updating from 6.0-RELEASE to
6.0-STABLE last friday
Hajimu UMEMOTO
ume at freebsd.org
Tue Nov 22 05:04:41 PST 2005
Hi,
>>>>> On Thu, 17 Nov 2005 11:24:16 -0500
>>>>> Pierre-Luc Drouin <pldrouin at pldrouin.net> said:
> Ok, there is new development. I realized by playing with
> debug.acpi.disabled="smbat", debug.acpi.disabled="smbat cmbat" and
> debug.acpi.disabled="cmbat", that my laptop battery is not a smbat,
> but a cmbat. When I played with hw.acpi.battery.info_expire after to
> have applied the patch for acpi_smbat.c, it was freezing less often
> because that sysctl variable was shared by both cmbat and smbat. So I
> can only get battery status from cmbat (disabling cmbat disables the
> use of acpiconf -i loop). To get the status of my battery via cmbat
> was working fine up to 6.0-RELEASE (included), but makes my laptop to
> freeze since I upgraded to 6.0-stable with Nov 10th sources. What
> change related to cmbat between 6.0-release and 6.0-stable could be
> causing this?
pldrouin> Has someone found how to fix this problem in -stable?
Perhaps, I found the cause. acpi_cmbat_get_bif() is heavy process,
and it was called only when ACPIIO_CMBAT_GET_BIF ioctl was issued
explicitly, until smbat stuff was committed. However,
acpiio_cmbat_get_bif() is called from every
acpi_battery_get_battinfo() call, now.
The attached patch will bring back to former behavior. Please try it
and let me know the result.
It is against 7-CURRENT as of today. If you want to try it on
6-STABLE, you need to apply following diff before applying it:
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/acpica/acpi_smbat.c.diff?r1=1.1&r2=1.2
Index: sys/dev/acpica/acpi_battery.c
diff -u -p sys/dev/acpica/acpi_battery.c.orig sys/dev/acpica/acpi_battery.c
--- sys/dev/acpica/acpi_battery.c.orig Mon Nov 7 01:12:11 2005
+++ sys/dev/acpica/acpi_battery.c Tue Nov 22 16:57:11 2005
@@ -178,7 +178,7 @@ acpi_battery_get_battinfo(device_t dev,
* return that the device is present.
*/
if (ACPI_BATT_GET_STATUS(batt_dev, &bst[i]) != 0 ||
- ACPI_BATT_GET_INFO(batt_dev, bif) != 0)
+ ACPI_BATT_GET_INFO(batt_dev, bif, TRUE) != 0)
continue;
/* If a battery is not installed, we sometimes get strange values. */
@@ -382,7 +382,7 @@ acpi_battery_ioctl(u_long cmd, caddr_t a
case ACPIIO_BATT_GET_BIF:
if (dev != NULL) {
bzero(&ioctl_arg->bif, sizeof(ioctl_arg->bif));
- error = ACPI_BATT_GET_INFO(dev, &ioctl_arg->bif);
+ error = ACPI_BATT_GET_INFO(dev, &ioctl_arg->bif, FALSE);
/*
* Remove invalid characters. Perhaps this should be done
Index: sys/dev/acpica/acpi_cmbat.c
diff -u -p sys/dev/acpica/acpi_cmbat.c.orig sys/dev/acpica/acpi_cmbat.c
--- sys/dev/acpica/acpi_cmbat.c.orig Sun Nov 20 12:23:21 2005
+++ sys/dev/acpica/acpi_cmbat.c Tue Nov 22 16:23:30 2005
@@ -83,7 +83,8 @@ static void acpi_cmbat_info_updated(str
static void acpi_cmbat_get_bst(device_t dev);
static void acpi_cmbat_get_bif(device_t dev);
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 int acpi_cmbat_bif(device_t dev, struct acpi_bif *bifp,
+ int cache);
static void acpi_cmbat_init_battery(void *arg);
static device_method_t acpi_cmbat_methods[] = {
@@ -354,14 +355,15 @@ end:
}
static int
-acpi_cmbat_bif(device_t dev, struct acpi_bif *bifp)
+acpi_cmbat_bif(device_t dev, struct acpi_bif *bifp, int cache)
{
struct acpi_cmbat_softc *sc;
sc = device_get_softc(dev);
ACPI_SERIAL_BEGIN(cmbat);
- acpi_cmbat_get_bif(dev);
+ if (!cache)
+ acpi_cmbat_get_bif(dev);
bifp->units = sc->bif.units;
bifp->dcap = sc->bif.dcap;
bifp->lfcap = sc->bif.lfcap;
Index: sys/dev/acpica/acpi_if.m
diff -u sys/dev/acpica/acpi_if.m.orig sys/dev/acpica/acpi_if.m
--- sys/dev/acpica/acpi_if.m.orig Sun Nov 20 12:23:21 2005
+++ sys/dev/acpica/acpi_if.m Tue Nov 22 16:58:09 2005
@@ -206,10 +206,12 @@
#
# device_t dev: Battery device
# struct acpi_bif *bif: Pointer to storage for _BIF results
+# int cache: if set, read from cache without any I/O to ACPI BIOS
#
METHOD int batt_get_info {
device_t dev;
struct acpi_bif *bif;
+ int cache;
};
#
Index: sys/dev/acpica/acpi_smbat.c
diff -u -p sys/dev/acpica/acpi_smbat.c.orig sys/dev/acpica/acpi_smbat.c
--- sys/dev/acpica/acpi_smbat.c.orig Tue Nov 22 16:07:38 2005
+++ sys/dev/acpica/acpi_smbat.c Tue Nov 22 18:37:13 2005
@@ -56,7 +56,8 @@ static int acpi_smbat_attach(device_t de
static int acpi_smbat_shutdown(device_t dev);
static int acpi_smbat_info_expired(struct timespec *lastupdated);
static void acpi_smbat_info_updated(struct timespec *lastupdated);
-static int acpi_smbat_get_bif(device_t dev, struct acpi_bif *bif);
+static int acpi_smbat_get_bif(device_t dev, struct acpi_bif *bif,
+ int cache);
static int acpi_smbat_get_bst(device_t dev, struct acpi_bst *bst);
ACPI_SERIAL_DECL(smbat, "ACPI Smart Battery");
@@ -124,6 +125,11 @@ acpi_smbat_attach(device_t dev)
timespecclear(&sc->bif_lastupdated);
timespecclear(&sc->bst_lastupdated);
+ if (acpi_smbat_get_bif(dev, NULL, FALSE) != 0) {
+ device_printf(dev, "cannot get battery information\n");
+ return (ENXIO);
+ }
+
if (acpi_battery_register(dev) != 0) {
device_printf(dev, "cannot register battery\n");
return (ENXIO);
@@ -381,7 +387,7 @@ out:
}
static int
-acpi_smbat_get_bif(device_t dev, struct acpi_bif *bif)
+acpi_smbat_get_bif(device_t dev, struct acpi_bif *bif, int cache)
{
struct acpi_smbat_softc *sc;
int error;
@@ -395,7 +401,7 @@ acpi_smbat_get_bif(device_t dev, struct
error = ENXIO;
sc = device_get_softc(dev);
- if (!acpi_smbat_info_expired(&sc->bif_lastupdated)) {
+ if (cache || !acpi_smbat_info_expired(&sc->bif_lastupdated)) {
error = 0;
goto out;
}
@@ -451,7 +457,8 @@ acpi_smbat_get_bif(device_t dev, struct
error = 0;
out:
- memcpy(bif, &sc->bif, sizeof(sc->bif));
+ if (bif != NULL)
+ memcpy(bif, &sc->bif, sizeof(sc->bif));
ACPI_SERIAL_END(smbat);
return (error);
}
Sincerely,
--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
ume at mahoroba.org ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/
More information about the freebsd-stable
mailing list