Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/cardbus convert cardslot event thread from wakeup/ts...



details:   https://anonhg.NetBSD.org/src/rev/8dae16bc5dc6
branches:  trunk
changeset: 378554:8dae16bc5dc6
user:      mrg <mrg%NetBSD.org@localhost>
date:      Sat Apr 17 01:19:48 2021 +0000

description:
convert cardslot event thread from wakeup/tsleep to mutex/condvar.
this avoids a strange hang at reboot i am seeing on an old pentium4-m
laptop, that was introduced with kern_mutex.c 1.91/92, though i can
not really explain why that matters (in the waiting thread, a pointer
that should be NULL remains non NULL.)  thanks to jmcneill@ for some
helpful review commentary here.

don't panic() if either "cardbus" or "pcmcia" didn't attach and a
card is inserted.  this can happen for various reasons, including
some regression in netbsd (-current, and -9, at least) that suggests
using PCI_BUS_FIXUP (though it still fails to attach the card i have.)

both found with GCC 10 testing, though both also occur with GCC 7 in
the netbsd-9 tree as well.

diffstat:

 sys/dev/cardbus/cardslot.c    |  61 ++++++++++++++++++++++++++----------------
 sys/dev/cardbus/cardslotvar.h |   6 +++-
 2 files changed, 43 insertions(+), 24 deletions(-)

diffs (180 lines):

diff -r 26be1e5cc9ba -r 8dae16bc5dc6 sys/dev/cardbus/cardslot.c
--- a/sys/dev/cardbus/cardslot.c        Sat Apr 17 00:05:31 2021 +0000
+++ b/sys/dev/cardbus/cardslot.c        Sat Apr 17 01:19:48 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cardslot.c,v 1.57 2020/10/04 06:15:54 nat Exp $        */
+/*     $NetBSD: cardslot.c,v 1.58 2021/04/17 01:19:48 mrg Exp $        */
 
 /*
  * Copyright (c) 1999 and 2000
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cardslot.c,v 1.57 2020/10/04 06:15:54 nat Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cardslot.c,v 1.58 2021/04/17 01:19:48 mrg Exp $");
 
 #include "opt_cardslot.h"
 
@@ -129,6 +129,8 @@ cardslotattach(device_t parent, device_t
        sc->sc_16_softc = NULL;
        SIMPLEQ_INIT(&sc->sc_events);
        sc->sc_th_enable = 0;
+       mutex_init(&sc->sc_event_lock, MUTEX_DEFAULT, IPL_TTY);
+       cv_init(&sc->sc_event_cv, "evexit");
 
        aprint_naive("\n");
        aprint_normal("\n");
@@ -192,14 +194,24 @@ cardslotdetach(device_t self, int flags)
        if ((rc = config_detach_children(self, flags)) != 0)
                return rc;
 
-       sc->sc_th_enable = 0;
-       wakeup(&sc->sc_events);
-       while (sc->sc_event_thread != NULL)
-               (void)tsleep(sc, PWAIT, "cardslotthd", 0);
+       mutex_enter(&sc->sc_event_lock);
+
+       if (sc->sc_event_thread != NULL) {
+               sc->sc_th_enable = 0;
+               cv_signal(&sc->sc_event_cv);
+               while (sc->sc_event_thread != NULL) {
+                       cv_wait(&sc->sc_event_cv, &sc->sc_event_lock);
+               }
+       }
 
        if (!SIMPLEQ_EMPTY(&sc->sc_events))
                aprint_error_dev(self, "events outstanding");
 
+       mutex_exit(&sc->sc_event_lock);
+
+       mutex_destroy(&sc->sc_event_lock);
+       cv_destroy(&sc->sc_event_cv);
+
        pmf_device_deregister(self);
        return 0;
 }
@@ -272,13 +284,11 @@ cardslot_event_throw(struct cardslot_sof
 
        ce->ce_type = ev;
 
-       {
-               int s = spltty();
-               SIMPLEQ_INSERT_TAIL(&sc->sc_events, ce, ce_q);
-               splx(s);
-       }
+       mutex_enter(&sc->sc_event_lock);
+       SIMPLEQ_INSERT_TAIL(&sc->sc_events, ce, ce_q);
+       mutex_exit(&sc->sc_event_lock);
 
-       wakeup(&sc->sc_events);
+       cv_signal(&sc->sc_event_cv);
 
        return;
 }
@@ -296,29 +306,28 @@ cardslot_event_thread(void *arg)
 {
        struct cardslot_softc *sc = arg;
        struct cardslot_event *ce;
-       int s, first = 1;
+       int first = 1;
        static int antonym_ev[4] = {
                CARDSLOT_EVENT_REMOVAL_16, CARDSLOT_EVENT_INSERTION_16,
                CARDSLOT_EVENT_REMOVAL_CB, CARDSLOT_EVENT_INSERTION_CB
        };
 
+       mutex_enter(&sc->sc_event_lock);
        while (sc->sc_th_enable) {
-               s = spltty();
                if ((ce = SIMPLEQ_FIRST(&sc->sc_events)) == NULL) {
-                       splx(s);
                        if (first) {
                                first = 0;
+                               mutex_exit(&sc->sc_event_lock);
                                config_pending_decr(sc->sc_dev);
+                               mutex_enter(&sc->sc_event_lock);
                        }
-                       (void) tsleep(&sc->sc_events, PWAIT, "cardslotev", 0);
+                       cv_wait(&sc->sc_event_cv, &sc->sc_event_lock);
                        continue;
                }
                SIMPLEQ_REMOVE_HEAD(&sc->sc_events, ce_q);
-               splx(s);
 
                if (IS_CARDSLOT_INSERT_REMOVE_EV(ce->ce_type)) {
                        /* Chattering suppression */
-                       s = spltty();
                        while (1) {
                                struct cardslot_event *ce1, *ce2;
 
@@ -340,8 +349,8 @@ cardslot_event_thread(void *arg)
                                        free(ce2, M_TEMP);
                                }
                        }
-                       splx(s);
                }
+               mutex_exit(&sc->sc_event_lock);
 
                switch (ce->ce_type) {
                case CARDSLOT_EVENT_INSERTION_CB:
@@ -371,8 +380,10 @@ cardslot_event_thread(void *arg)
                                            CARDSLOT_STATUS_NOTWORK);
                                }
                        } else {
-                               panic("no cardbus on %s",
+                               printf("%s: no cardbus on %s\n", __func__,
                                      device_xname(sc->sc_dev));
+                               CARDSLOT_SET_WORK(sc->sc_status,
+                                   CARDSLOT_STATUS_NOTWORK);
                        }
 
                        break;
@@ -400,8 +411,10 @@ cardslot_event_thread(void *arg)
                                            CARDSLOT_STATUS_WORKING);
                                }
                        } else {
-                               panic("no 16-bit pcmcia on %s",
+                               printf("%s: no 16-bit pcmcia on %s\n", __func__,
                                      device_xname(sc->sc_dev));
+                               CARDSLOT_SET_WORK(sc->sc_status,
+                                   CARDSLOT_STATUS_NOTWORK);
                        }
 
                        break;
@@ -451,12 +464,14 @@ cardslot_event_thread(void *arg)
                        panic("cardslot_event_thread: unknown event %d", ce->ce_type);
                }
                free(ce, M_TEMP);
+               mutex_enter(&sc->sc_event_lock);
        }
 
+       /* The parent device is waiting for us to exit. */
        sc->sc_event_thread = NULL;
 
-       /* In case the parent device is waiting for us to exit. */
-       wakeup(sc);
+       cv_signal(&sc->sc_event_cv);
+       mutex_exit(&sc->sc_event_lock);
 
        kthread_exit(0);
 }
diff -r 26be1e5cc9ba -r 8dae16bc5dc6 sys/dev/cardbus/cardslotvar.h
--- a/sys/dev/cardbus/cardslotvar.h     Sat Apr 17 00:05:31 2021 +0000
+++ b/sys/dev/cardbus/cardslotvar.h     Sat Apr 17 01:19:48 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cardslotvar.h,v 1.16 2009/12/15 22:17:12 snj Exp $     */
+/*     $NetBSD: cardslotvar.h,v 1.17 2021/04/17 01:19:48 mrg Exp $     */
 
 /*
  * Copyright (c) 1999
@@ -65,6 +65,10 @@ struct cardslot_softc {
        /* An event queue for the thread which processes slot state events. */
 
        SIMPLEQ_HEAD(, cardslot_event) sc_events;
+
+       /* Safely handle detach. */
+       kmutex_t sc_event_lock;
+       kcondvar_t sc_event_cv;
 };
 
 #define CARDSLOT_STATUS_CARD_MASK     0x07



Home | Main Index | Thread Index | Old Index