Date: Tue, 22 Nov 2005 22:04:28 +0900 From: Hajimu UMEMOTO <ume@freebsd.org> To: Pierre-Luc Drouin <pldrouin@pldrouin.net> Cc: acpi@freebsd.org, freebsd-stable@freebsd.org Subject: Re: Performance problem since updating from 6.0-RELEASE to 6.0-STABLE last friday Message-ID: <yge7jb0x28z.wl%ume@mahoroba.org> In-Reply-To: <437CAEB0.9060202@pldrouin.net> References: <4377775B.3080606@pldrouin.net> <20051114105854.GA1041@galgenberg.net> <4378CC14.2020109@pldrouin.net> <ygek6f9g83g.wl%ume@mahoroba.org> <437A3901.8010001@pldrouin.net> <437A3B96.4040300@root.org> <437B42C0.9040605@pldrouin.net> <437CAEB0.9060202@pldrouin.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi, >>>>> On Thu, 17 Nov 2005 11:24:16 -0500 >>>>> Pierre-Luc Drouin <pldrouin@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@mahoroba.org ume@{,jp.}FreeBSD.org http://www.imasy.org/~ume/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?yge7jb0x28z.wl%ume>