Kernel panic when unpluggin AC adaptor

[Giovanni Trematerra]

On Thu, May 13, 2010 at 1:09 AM, Brandon Gooch
<jamesbrandongooch at gmail.com> wrote:
> On Wed, May 12, 2010 at 9:41 AM, Attilio Rao <attilio at freebsd.org> wrote:
>> 2010/5/12 David DEMELIER <demelier.david at gmail.com>:
>>> I remove the patch, and built the kernel (I updated the src this
>>> morning) and it does not panic now. It's really odd. If it reappears
>>> soon I will tell you.
>>
>> I looked at the code with Giovanni and I have the feeling that the
>> race with the idle thread may still be fatal.
>> We need to fix that.
>>
>> Attilio
>>
>
> That seems to be the case, as my laptop shows about an 80-85 % chance
> of experiencing a panic if left idle for long-ish periods of time (2
> to 4 hours). I usually rebuild world or big ports overnight, and more
> often than not I wake up to a panicked machine, same situation every
> time:
>
> ...
> rman_get_bushandle() at rman_get_bushandle+0x1
> sched_idletd() at sched_idletd+0x123
> fork_exit() at fork_exit+0x12a
> fork_trampoline() at fork_trampoline+0xe
> ...
>
> The kernel/userland is rebuilt, the ports are finished compiling --
> it's in the time AFTER the completion of all tasks that the machine
> gets bored and tries to kill itself :)
>
> I have seen the AC adapter plug/unplug "hang" in the past on this
> laptop, but I never made the connection between the events, as
> nowadays my laptop usually stays plugged in :(
>
> Attilio, I hope you can track this one down, let me know if I can do
> anything to help or test...
>

Attilio and I came up with this patch. It seems ready for stress
testing and review
Please test and report back.

Thank you

P.S: all the faults are only mine.

--
Gianni
-------------- next part --------------
diff -r d7d0e04f42e3 sys/dev/acpica/acpi_cpu.c
--- a/sys/dev/acpica/acpi_cpu.c	Wed May 12 04:01:56 2010 +0200
+++ b/sys/dev/acpica/acpi_cpu.c	Fri May 14 02:20:18 2010 +0200
@@ -88,6 +88,7 @@ struct acpi_cpu_softc {
     int			 cpu_cx_lowest;
     char 		 cpu_cx_supported[64];
     int			 cpu_rid;
+	struct mtx	 cpu_lock;
 };
 
 struct acpi_cpu_device {
@@ -100,6 +101,10 @@ struct acpi_cpu_device {
 #define CPU_SET_REG(reg, width, val)					\
     (bus_space_write_ ## width(rman_get_bustag((reg)), 			\
 		       rman_get_bushandle((reg)), 0, (val)))
+#define ACPI_CPU_LOCK(sc) \
+	mtx_lock_spin(&sc->cpu_lock)
+#define ACPI_CPU_UNLOCK(sc) \
+	mtx_unlock_spin(&sc->cpu_lock)
 
 #define PM_USEC(x)	 ((x) >> 2)	/* ~4 clocks per usec (3.57955 Mhz) */
 
@@ -284,6 +289,7 @@ acpi_cpu_attach(device_t dev)
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
     sc = device_get_softc(dev);
+	mtx_init(&sc->cpu_lock, "ntflck", NULL, MTX_SPIN);
     sc->cpu_dev = dev;
     sc->cpu_handle = acpi_get_handle(dev);
     cpu_id = (int)(intptr_t)acpi_get_private(dev);
@@ -409,26 +415,26 @@ acpi_cpu_postattach(void *unused __unuse
 SYSINIT(acpi_cpu, SI_SUB_CONFIGURE, SI_ORDER_MIDDLE,
     acpi_cpu_postattach, NULL);
 
-/*
- * Disable any entry to the idle function during suspend and re-enable it
- * during resume.
- */
 static int
 acpi_cpu_suspend(device_t dev)
 {
+    struct acpi_cpu_softc *sc;
     int error;
 
+    sc = device_get_softc(dev);
     error = bus_generic_suspend(dev);
     if (error)
 	return (error);
+	ACPI_CPU_LOCK(sc);
     cpu_disable_idle = TRUE;
+	ACPI_CPU_UNLOCK(sc);
+
     return (0);
 }
 
 static int
 acpi_cpu_resume(device_t dev)
 {
-
     cpu_disable_idle = FALSE;
     return (bus_generic_resume(dev));
 }
@@ -609,7 +615,9 @@ acpi_cpu_generic_cx_probe(struct acpi_cp
 	    cx_ptr->trans_lat = AcpiGbl_FADT.C2Latency;
 	    cx_ptr++;
 	    sc->cpu_cx_count++;
-	}
+	} else
+		panic("%s: Cannot allocate resource %d for C3 state", __func__, 
+		    cx_ptr->res_type);
     }
     if (sc->cpu_p_blk_len < 6)
 	return;
@@ -625,7 +633,9 @@ acpi_cpu_generic_cx_probe(struct acpi_cp
 	    cx_ptr->trans_lat = AcpiGbl_FADT.C3Latency;
 	    cx_ptr++;
 	    sc->cpu_cx_count++;
-	}
+	} else
+		panic("%s: Cannot allocate resource %d for C3 state", __func__, 
+		    cx_ptr->res_type);
     }
 }
 
@@ -721,6 +731,8 @@ acpi_cpu_cx_cst(struct acpi_cpu_softc *s
 	}
 #endif
 
+	ACPI_CPU_LOCK(sc);
+
 	/* Allocate the control register for C2 or C3. */
 	acpi_PkgGas(sc->cpu_dev, pkg, 0, &cx_ptr->res_type, &sc->cpu_rid,
 	    &cx_ptr->p_lvlx, RF_SHAREABLE);
@@ -732,7 +744,17 @@ acpi_cpu_cx_cst(struct acpi_cpu_softc *s
 			     cx_ptr->trans_lat));
 	    cx_ptr++;
 	    sc->cpu_cx_count++;
+		cpu_disable_idle = FALSE;
+	} else {
+		device_printf(sc->cpu_dev, "cannot allocate control register"
+		    " for C2 o C3.");
+
+		/*
+		 * disable acpi_cpu_idle otherwise we get a panic
+		 */
+		cpu_disable_idle = TRUE;
 	}
+	ACPI_CPU_UNLOCK(sc);
     }
     AcpiOsFree(buf.Pointer);
 
@@ -900,6 +923,15 @@ acpi_cpu_idle()
 	return;
     }
 
+	ACPI_CPU_LOCK(sc);
+
+    /* in the meantime acpi_cpu_notify could be disabled the hook */
+    if (cpu_disable_idle) {
+	ACPI_CPU_UNLOCK(sc);
+	ACPI_ENABLE_IRQS();
+	return;
+    }
+	
     /* Find the lowest state that has small enough latency. */
     cx_next_idx = 0;
     for (i = sc->cpu_cx_lowest; i >= 0; i--) {
@@ -935,6 +967,7 @@ acpi_cpu_idle()
      */
     if (cx_next->type == ACPI_STATE_C1) {
 	sc->cpu_prev_sleep = (sc->cpu_prev_sleep * 3 + 500000 / hz) / 4;
+	ACPI_CPU_UNLOCK(sc);
 	acpi_cpu_c1();
 	return;
     }
@@ -975,6 +1008,7 @@ acpi_cpu_idle()
 	AcpiWriteBitRegister(ACPI_BITREG_ARB_DISABLE, 0);
 	AcpiWriteBitRegister(ACPI_BITREG_BUS_MASTER_RLD, 0);
     }
+	ACPI_CPU_UNLOCK(sc);
     ACPI_ENABLE_IRQS();
 
     /* Find the actual time asleep in microseconds. */