Kernel panic when unpluggin AC adaptor

Giovanni Trematerra giovanni.trematerra at gmail.com
Wed May 26 13:55:53 UTC 2010


On Wed, May 26, 2010 at 3:55 PM, Giovanni Trematerra
<giovanni.trematerra at gmail.com> wrote:
> On Wed, May 26, 2010 at 12:01 PM, David DEMELIER
> <demelier.david at gmail.com> wrote:
>> 2010/5/26 Giovanni Trematerra <giovanni.trematerra at gmail.com>:
>>> On Wed, May 26, 2010 at 11:14 AM, David DEMELIER
>>> <demelier.david at gmail.com> wrote:
>>>> 2010/5/25 Giovanni Trematerra <giovanni.trematerra at gmail.com>:
>>>>> On Tue, May 25, 2010 at 5:52 PM, David DEMELIER
>>>>> <demelier.david at gmail.com> wrote:
>>>>>> 2010/5/25 Giovanni Trematerra <giovanni.trematerra at gmail.com>:
>>>>>>> On Mon, May 24, 2010 at 9:43 PM, David DEMELIER
>>>>>>> <demelier.david at gmail.com> wrote:
>>>>>>>> 2010/5/12 Giovanni Trematerra <giovanni.trematerra at gmail.com>:
>>>>>>>>> On Fri, May 7, 2010 at 8:33 PM, Demelier David <demelier.david at gmail.com> wrote:
>>>>>>>>>> Le Vendredi 07 mai 2010 à 18:22 +0200, Giovanni Trematerra a écrit :
>>>>>>>>>>> On Fri, May 7, 2010 at 2:08 PM, Demelier David <demelier.david at gmail.com> wrote:
>>>>>>>>>>> > Hi,
>>>>>>>>>>> >        I noticed that pluggin the AC adaptor when I boot without it does not
>>>>>>>>>>> >        panic. It only panic when removing it.
>>>>>>>>>>> >
>>>>>>>>>>> >        Maybe that could help ?
>>>>>>>>>>> >
>>>>>>>>>>>
>>>>>>>>>>> Good to know. The problem lies somewhere when performance state change.
>>>>>>>>>>> In your case it happens when you remove AC adaptor. Let's hope someone on
>>>>>>>>>>> acpi@ ml comes up with a good idea.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Okay so for the moment no change, I'll wait for someone with an idea
>>>>>>>>>> that could solve my problem. For me because the panic only happens when
>>>>>>>>>> changing profile from ac plugged -> ac unplugged (and not the reverse) I
>>>>>>>>>> would think it's a cpu related acpi issue.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I looked deeper and it seems to me that when you unplug the AC
>>>>>>>>> adapter, acpi_cpu_notify calls acpi_cpu_cx_cst that try to allocate a
>>>>>>>>> new cx_ptr->p_lvlx  via acpi_PkgGas.
>>>>>>>>> If acpi_PkgGas set cx_ptr->p_lvlx to NULL for any reasons you'll have
>>>>>>>>> the panic that you reported.
>>>>>>>>> A solution would be to set acpi_cpu_hook to NULL so acpi_cpu_idle won't call it.
>>>>>>>>> I need some time to have a patch because of the possible race between
>>>>>>>>> acpi_cpu_notify and
>>>>>>>>> acpi_cpu_idle during set acpi_cpu_hook to NULL.
>>>>>>>>> if you have time and want panic your system you could try the attached
>>>>>>>>> patch, just to be
>>>>>>>>> sure that we catch it.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Hi, it paniced today ! I don't know why it randomly panic but it did,
>>>>>>>> the backtrace didn't change. There is a picture about the panic :
>>>>>>>>
>>>>>>>> http://img541.imageshack.us/img541/2773/dsc00388xa.jpg
>>>>>>>
>>>>>>> What was you trying? acpi_idle5.diff.txt patch?
>>>>>>> How did it panic? Unplugging AC adapter?
>>>>>>>
>>>>>>
>>>>>> Hi, I tried this one : lvlx.diff.txt. Yes by unplugging the AC adapter.
>>>>>>
>>>>>
>>>>> This is an old one. Could you try acpi_idle5.diff.txt? I kept you in
>>>>> Cc when I sent to
>>>>> the list. If you have problems, let me know, I'll resend to you  the patch.
>>>>>
>>>>> Thank you.
>>>>>
>>>>
>>>> Hi, it panic'ed with the same backtrace.
>>>>
>>>
>>> Can you please post your dmesg?
>>>
>>
>> Sent !
>
> As your PC is in a good mood to make test, :)
> could you try this patch?
>
> Thank you
>
> --
> Gianni
>

Here the patch :(
-------------- next part --------------
diff -r ac95a73d358d sys/dev/acpica/acpi_cpu.c
--- a/sys/dev/acpica/acpi_cpu.c	Tue May 18 08:13:40 2010 -0400
+++ b/sys/dev/acpica/acpi_cpu.c	Wed May 26 09:44:49 2010 -0400
@@ -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) */
 
@@ -127,7 +132,7 @@ static int		 cpu_quirks;	/* Indicate any
 static int		 cpu_quirks;	/* Indicate any hardware bugs. */
 
 /* Runtime state. */
-static int		 cpu_disable_idle; /* Disable entry to idle function */
+static int		 cpu_disable_idle; /* Global disable 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);
@@ -416,20 +422,25 @@ static int
 static int
 acpi_cpu_suspend(device_t dev)
 {
+    struct acpi_cpu_softc *sc;
     int error;
 
+    sc = device_get_softc(dev);
+    ACPI_CPU_LOCK(sc);
+    cpu_disable_idle = TRUE;
+    ACPI_CPU_UNLOCK(sc);
     error = bus_generic_suspend(dev);
     if (error)
 	return (error);
-    cpu_disable_idle = TRUE;
+
     return (0);
 }
 
 static int
 acpi_cpu_resume(device_t dev)
 {
+    cpu_disable_idle = FALSE;
 
-    cpu_disable_idle = FALSE;
     return (bus_generic_resume(dev));
 }
 
@@ -523,16 +534,15 @@ acpi_cpu_shutdown(device_t dev)
 {
     ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__);
 
+    struct acpi_cpu_softc *sc;
+
+    sc = device_get_softc(dev);
+    ACPI_CPU_LOCK(sc);
+    cpu_disable_idle = TRUE;
+    ACPI_CPU_UNLOCK(sc);
+
     /* 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;
 
     return_VALUE (0);
 }
@@ -609,7 +619,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 +637,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 +651,14 @@ static int
 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 +737,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.
+	 */
+	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 +757,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 or C3.");
+	ACPI_CPU_UNLOCK(sc);
     }
     AcpiOsFree(buf.Pointer);
 
@@ -883,12 +911,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
      * since there is no ACPI processor object for this CPU.  This occurs
@@ -896,7 +918,17 @@ acpi_cpu_idle()
      */
     sc = cpu_softc[PCPU_GET(cpuid)];
     if (sc == NULL) {
-	acpi_cpu_c1();
+	if (cpu_disable_idle)
+	    ACPI_ENABLE_IRQS();
+	else
+	    acpi_cpu_c1();
+	return;
+    }
+
+    ACPI_CPU_LOCK(sc);
+    if (cpu_disable_idle) {
+	ACPI_CPU_UNLOCK(sc);
+	ACPI_ENABLE_IRQS();
 	return;
     }
 
@@ -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. */


More information about the freebsd-stable mailing list