On Sat, May 15, 2010 at 9:12 PM, Brandon Gooch
<jamesbrandongo...@gmail.com> wrote:
> On Thu, May 13, 2010 at 7:25 PM, Giovanni Trematerra
> <giovanni.tremate...@gmail.com> wrote:
>> On Thu, May 13, 2010 at 1:09 AM, Brandon Gooch
>> <jamesbrandongo...@gmail.com> wrote:
>>> On Wed, May 12, 2010 at 9:41 AM, Attilio Rao <atti...@freebsd.org> wrote:
>>>> 2010/5/12 David DEMELIER <demelier.da...@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
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. */
_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Reply via email to