Kernel panic when unpluggin AC adaptor

Giovanni Trematerra giovanni.trematerra at gmail.com
Tue May 18 14:04:09 UTC 2010


On Sat, May 15, 2010 at 9:12 PM, Brandon Gooch
<jamesbrandongooch at gmail.com> wrote:
> On Thu, May 13, 2010 at 7:25 PM, Giovanni Trematerra
> <giovanni.trematerra at gmail.com> wrote:
>> 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.
>
> I tried the patch, and my kernel panics I panic on boot. I have
> 8.5MB(!) of JPG images (6 of them) if anyone needs to see them. I'm
> looking for a place to post them, but if anyone wants, I can send via
> e-mail...

Hi Brandon,
Could you please, try this new one? The panic at boot stage should be solved,
at least I tried on a 8-way machine and all went ok at boot.
Please, remove WITNESS_SKIPSPIN from your kernel config file.
This patch might be sub-optimal and contains style(9) error but if it
works we are
on the right way.
Let me know if it works for you.

Thanks

--
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	Mon May 17 09:21:25 2010 +0200
@@ -88,6 +88,8 @@ struct acpi_cpu_softc {
     int			 cpu_cx_lowest;
     char 		 cpu_cx_supported[64];
     int			 cpu_rid;
+	struct mtx	 cpu_lock;
+	int			 cpu_disable_idle;
 };
 
 struct acpi_cpu_device {
@@ -100,6 +102,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) */
 
@@ -127,7 +133,6 @@ static uint8_t		 cpu_cst_cnt;	/* Indicat
 static int		 cpu_quirks;	/* Indicate any hardware bugs. */
 
 /* Runtime state. */
-static int		 cpu_disable_idle; /* Disable entry to idle function */
 static int		 cpu_cx_count;	/* Number of valid Cx states */
 
 /* Values for sysctl. */
@@ -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,27 +415,30 @@ 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);
-    cpu_disable_idle = TRUE;
+	ACPI_CPU_LOCK(sc);
+    sc->cpu_disable_idle = TRUE;
+	ACPI_CPU_UNLOCK(sc);
+
     return (0);
 }
 
 static int
 acpi_cpu_resume(device_t dev)
 {
+    struct acpi_cpu_softc *sc;
 
-    cpu_disable_idle = FALSE;
+    sc = device_get_softc(dev);
+    sc->cpu_disable_idle = FALSE;
     return (bus_generic_resume(dev));
 }
 
@@ -523,16 +532,16 @@ acpi_cpu_shutdown(device_t dev)
 {
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
+    struct acpi_cpu_softc *sc;
+
+    sc = device_get_softc(dev);
+
     /* Allow children to shutdown first. */
     bus_generic_shutdown(dev);
 
-    /*
-     * Disable any entry to the idle function.  There is a small race where
-     * an idle thread have passed this check but not gone to sleep.  This
-     * is ok since device_shutdown() does not free the softc, otherwise
-     * we'd have to be sure all threads were evicted before returning.
-     */
-    cpu_disable_idle = TRUE;
+	ACPI_CPU_LOCK(sc);
+    sc->cpu_disable_idle = TRUE;
+	ACPI_CPU_UNLOCK(sc);
 
     return_VALUE (0);
 }
@@ -609,7 +618,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 +636,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);
     }
 }
 
@@ -637,13 +650,14 @@ acpi_cpu_generic_cx_probe(struct acpi_cp
 static int
 acpi_cpu_cx_cst(struct acpi_cpu_softc *sc)
 {
+    struct 	 resource *lvlx;
     struct	 acpi_cx *cx_ptr;
     ACPI_STATUS	 status;
     ACPI_BUFFER	 buf;
     ACPI_OBJECT	*top;
     ACPI_OBJECT	*pkg;
     uint32_t	 count;
-    int		 i;
+    int		 i, type, rid;
 
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
@@ -722,8 +736,18 @@ acpi_cpu_cx_cst(struct acpi_cpu_softc *s
 #endif
 
 	/* 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);
+	acpi_PkgGas(sc->cpu_dev, pkg, 0, &type, &rid,
+	    &lvlx, RF_SHAREABLE);
+	ACPI_CPU_LOCK(sc);
+
+	/* 
+	 *  if you cannot allocate the control register you need to stop
+	 *  the acpi_cpu_idle hook.
+	 */
+	sc->cpu_disable_idle = (lvlx == NULL) ? TRUE : FALSE;
+	sc->cpu_rid  	 = rid;
+	cx_ptr->p_lvlx   = lvlx;
+	cx_ptr->res_type = type;
 	if (cx_ptr->p_lvlx) {
 	    sc->cpu_rid++;
 	    ACPI_DEBUG_PRINT((ACPI_DB_INFO,
@@ -732,7 +756,10 @@ acpi_cpu_cx_cst(struct acpi_cpu_softc *s
 			     cx_ptr->trans_lat));
 	    cx_ptr++;
 	    sc->cpu_cx_count++;
-	}
+	} else
+		device_printf(sc->cpu_dev, "cannot allocate control register"
+		    " for C2 o C3.");
+	ACPI_CPU_UNLOCK(sc);
     }
     AcpiOsFree(buf.Pointer);
 
@@ -813,7 +840,10 @@ acpi_cpu_startup(void *arg)
 
     /* Take over idling from cpu_idle_default(). */
     cpu_cx_lowest = 0;
-    cpu_disable_idle = FALSE;
+	for (i = 0; i < cpu_ndevices; i++) {
+	    sc = device_get_softc(cpu_devices[i]);
+    	sc->cpu_disable_idle = FALSE;
+	}
     cpu_idle_hook = acpi_cpu_idle;
 }
 
@@ -883,11 +913,6 @@ acpi_cpu_idle()
     uint32_t	start_time, end_time;
     int		bm_active, cx_next_idx, i;
 
-    /* If disabled, return immediately. */
-    if (cpu_disable_idle) {
-	ACPI_ENABLE_IRQS();
-	return;
-    }
 
     /*
      * Look up our CPU id to get our softc.  If it's NULL, we'll use C1
@@ -900,6 +925,14 @@ acpi_cpu_idle()
 	return;
     }
 
+    ACPI_CPU_LOCK(sc);
+
+    if (sc->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 +968,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 +1009,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. */


More information about the freebsd-stable mailing list