Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 14 Mar 2017 15:58:01 +0000 (UTC)
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r315266 - stable/8/sys/dev/acpica
Message-ID:  <201703141558.v2EFw1Wn017891@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: hselasky
Date: Tue Mar 14 15:58:01 2017
New Revision: 315266
URL: https://svnweb.freebsd.org/changeset/base/315266

Log:
  MFC r314328:
  
  Fix startup race initialising ACPI CM battery structures on MacBookPro.
  
  During acpi_cmbat_attach() the acpi_cmbat_init_battery() notification
  handler is registered. It has been observed this notification handler
  can be called instantly, before the attach routine has returned. In
  the notification handler there is a call to device_is_attached() which
  returns false. Because the softc is set we know an attach is in
  progress and the fix is simply to wait and try again in this case.
  
  Reviewed by:		avg @

Modified:
  stable/8/sys/dev/acpica/acpi_cmbat.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/dev/   (props changed)
  stable/8/sys/dev/acpica/   (props changed)

Modified: stable/8/sys/dev/acpica/acpi_cmbat.c
==============================================================================
--- stable/8/sys/dev/acpica/acpi_cmbat.c	Tue Mar 14 15:57:21 2017	(r315265)
+++ stable/8/sys/dev/acpica/acpi_cmbat.c	Tue Mar 14 15:58:01 2017	(r315266)
@@ -163,6 +163,16 @@ acpi_cmbat_detach(device_t dev)
     handle = acpi_get_handle(dev);
     AcpiRemoveNotifyHandler(handle, ACPI_ALL_NOTIFY, acpi_cmbat_notify_handler);
     acpi_battery_remove(dev);
+
+    /*
+     * Force any pending notification handler calls to complete by
+     * requesting cmbat serialisation while freeing and clearing the
+     * softc pointer:
+     */
+    ACPI_SERIAL_BEGIN(cmbat);
+    device_set_softc(dev, NULL);
+    ACPI_SERIAL_END(cmbat);
+
     return (0);
 }
 
@@ -435,7 +445,6 @@ acpi_cmbat_init_battery(void *arg)
     device_t	dev;
 
     dev = (device_t)arg;
-    sc = device_get_softc(dev);
     ACPI_VPRINT(dev, acpi_device_get_parent_softc(dev),
 		"battery initialization start\n");
 
@@ -445,18 +454,33 @@ acpi_cmbat_init_battery(void *arg)
      * to wait a while.
      */
     for (retry = 0; retry < ACPI_CMBAT_RETRY_MAX; retry++, AcpiOsSleep(10000)) {
-	/* batteries on DOCK can be ejected w/ DOCK during retrying */
-	if (!device_is_attached(dev))
+	/*
+	 * Batteries on DOCK can be ejected w/ DOCK during retrying.
+	 *
+	 * If there is a valid softc pointer the device may be in
+	 * attaching, attached or detaching state. If the state is
+	 * different from attached retry getting the device state
+	 * until it becomes stable. This solves a race if the ACPI
+	 * notification handler is called during attach, because
+	 * device_is_attached() doesn't return non-zero until after
+	 * the attach code has been executed.
+	 */
+	ACPI_SERIAL_BEGIN(cmbat);
+	sc = device_get_softc(dev);
+	if (sc == NULL) {
+	    ACPI_SERIAL_END(cmbat);
 	    return;
+	}
 
-	if (!acpi_BatteryIsPresent(dev))
+	if (!acpi_BatteryIsPresent(dev) || !device_is_attached(dev)) {
+	    ACPI_SERIAL_END(cmbat);
 	    continue;
+	}
 
 	/*
 	 * Only query the battery if this is the first try or the specific
 	 * type of info is still invalid.
 	 */
-	ACPI_SERIAL_BEGIN(cmbat);
 	if (retry == 0 || !acpi_battery_bst_valid(&sc->bst)) {
 	    timespecclear(&sc->bst_lastupdated);
 	    acpi_cmbat_get_bst(dev);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703141558.v2EFw1Wn017891>