NetBSD-Bugs archive

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

Re: kern/43125 (zyd(4) crashes with Buffalo WLI-U2-KG54L)



> Synopsis: zyd(4) crashes with Buffalo WLI-U2-KG54L

> I can repeat the problem with a SMC USB2.0 WLAN and have a patch.

Any progress on this?

> State-Changed-From-To: open->analyzed
 :
> zyd (and probably rum) do some magic to delay firmware installation
> until the device is initialized. However, you can call ioctl which
> then uses uninitialized data structures.

FUKAUMI Naoki suggested on tech-kern@ to introduce aftermountroothook()
to handle deferred initialization that should be done after mountroot():
http://mail-index.NetBSD.org/tech-kern/2010/05/27/msg008261.html

But as I wrote in the thread, I think it's rather better to handle
such device initializations in the autoconf(9) layer as we do
in config_interrupt(9) than adding a common hook because
such MI hooks seem deprecated like shutdownhook_establish(9) etc.

How about this patch that introduces config_mountroot(9)?
(zyd diff is based on FUKAUMI-san's one)

---
Index: dev/usb/if_zyd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/if_zyd.c,v
retrieving revision 1.25
diff -u -r1.25 if_zyd.c
--- dev/usb/if_zyd.c    5 Apr 2010 07:21:49 -0000       1.25
+++ dev/usb/if_zyd.c    13 Jun 2010 03:00:12 -0000
@@ -156,7 +156,7 @@
 CFATTACH_DECL_NEW(zyd, sizeof(struct zyd_softc), zyd_match,
     zyd_attach, zyd_detach, zyd_activate);
 
-Static int     zyd_attachhook(void *);
+Static void    zyd_attachhook(device_t);
 Static int     zyd_complete_attach(struct zyd_softc *);
 Static int     zyd_open_pipes(struct zyd_softc *);
 Static void    zyd_close_pipes(struct zyd_softc *);
@@ -245,10 +245,10 @@
            UMATCH_VENDOR_PRODUCT : UMATCH_NONE;
 }
 
-Static int
-zyd_attachhook(void *xsc)
+Static void
+zyd_attachhook(device_t self)
 {
-       struct zyd_softc *sc = xsc;
+       struct zyd_softc *sc = device_private(self);
        firmware_handle_t fwh;
        const char *fwname;
        u_char *fw;
@@ -259,7 +259,7 @@
        if ((error = firmware_open("zyd", fwname, &fwh)) != 0) {
                aprint_error_dev(sc->sc_dev,
                    "failed to open firmware %s (error=%d)\n", fwname, error);
-               return error;
+               return;
        }
        size = firmware_get_size(fwh);
        fw = firmware_malloc(size);
@@ -267,7 +267,7 @@
                aprint_error_dev(sc->sc_dev,
                    "failed to allocate firmware memory\n");
                firmware_close(fwh);
-               return ENOMEM;
+               return;
        }
        error = firmware_read(fwh, 0, fw, size);
        firmware_close(fwh);
@@ -275,7 +275,7 @@
                aprint_error_dev(sc->sc_dev,
                    "failed to read firmware (error %d)\n", error);
                firmware_free(fw, 0);
-               return error;
+               return;
        }
 
        error = zyd_loadfirmware(sc, fw, size);
@@ -283,7 +283,7 @@
                aprint_error_dev(sc->sc_dev,
                    "could not load firmware (error=%d)\n", error);
                firmware_free(fw, 0);
-               return ENXIO;
+               return;
        }
 
        firmware_free(fw, 0);
@@ -292,7 +292,7 @@
        /* complete the attach process */
        if ((error = zyd_complete_attach(sc)) == 0)
                sc->attached = 1;
-       return error;
+       return;
 }
 
 void
@@ -334,13 +334,10 @@
        IFQ_SET_READY(&ifp->if_snd);
        memcpy(ifp->if_xname, device_xname(sc->sc_dev), IFNAMSIZ);
 
-       if_attach(ifp);
-       /* XXXX: alloc temporarily until the layer2 can be configured. */
-       if_alloc_sadl(ifp);
-
        SIMPLEQ_INIT(&sc->sc_rqh);
 
-       return;
+       /* defer configrations after file system is ready to load firmware */
+       config_mountroot(self, zyd_attachhook);
 }
 
 Static int
@@ -424,7 +421,7 @@
                    IEEE80211_CHAN_DYN | IEEE80211_CHAN_2GHZ;
        }
 
-       if_free_sadl(ifp);
+       if_attach(ifp);
        ieee80211_ifattach(ic);
        ic->ic_node_alloc = zyd_node_alloc;
        ic->ic_newassoc = zyd_newassoc;
@@ -461,11 +458,8 @@
        struct ifnet *ifp = &sc->sc_if;
        int s;
 
-       if (!sc->attached) {
-               if_free_sadl(ifp);
-               if_detach(ifp);
+       if (!sc->attached)
                return 0;
-       }
 
        s = splusb();
 
@@ -2450,10 +2444,6 @@
        struct ieee80211com *ic = &sc->sc_ic;
        int i, error;
 
-       if ((sc->sc_flags & ZD1211_FWLOADED) == 0)
-               if ((error = zyd_attachhook(sc)) != 0)
-                       return error;
-
        zyd_stop(ifp, 0);
 
        IEEE80211_ADDR_COPY(ic->ic_myaddr, CLLADDR(ifp->if_sadl));
Index: kern/init_main.c
===================================================================
RCS file: /cvsroot/src/sys/kern/init_main.c,v
retrieving revision 1.420
diff -u -r1.420 init_main.c
--- kern/init_main.c    10 Jun 2010 20:54:53 -0000      1.420
+++ kern/init_main.c    13 Jun 2010 03:00:12 -0000
@@ -266,6 +266,7 @@
 static void start_init(void *);
 static void configure(void);
 static void configure2(void);
+static void configure3(void);
 void main(void);
 
 /*
@@ -626,6 +627,8 @@
        } while (error != 0);
        mountroothook_destroy();
 
+       configure3();
+
        /*
         * Initialise the time-of-day clock, passing the time recorded
         * in the root filesystem (if any) for use by systems that
@@ -783,6 +786,20 @@
 }
 
 static void
+configure3(void)
+{
+
+       /*
+        * Create threads to call back and finish configuration for
+        * devices that want the mounted root file system.
+        */
+       config_create_mountrootthreads();
+
+       /* Get the threads going and into any sleeps before continuing. */
+       yield();
+}
+
+static void
 check_console(struct lwp *l)
 {
        struct vnode *vp;
Index: kern/subr_autoconf.c
===================================================================
RCS file: /cvsroot/src/sys/kern/subr_autoconf.c,v
retrieving revision 1.206
diff -u -r1.206 subr_autoconf.c
--- kern/subr_autoconf.c        30 Apr 2010 21:17:22 -0000      1.206
+++ kern/subr_autoconf.c        13 Jun 2010 03:00:12 -0000
@@ -200,6 +200,9 @@
 struct deferred_config_head interrupt_config_queue =
        TAILQ_HEAD_INITIALIZER(interrupt_config_queue);
 int interrupt_config_threads = 8;
+struct deferred_config_head mountroot_config_queue =
+       TAILQ_HEAD_INITIALIZER(mountroot_config_queue);
+int mountroot_config_threads = 2;
 
 static void config_process_deferred(struct deferred_config_head *, device_t);
 
@@ -427,6 +430,7 @@
 {
        config_process_deferred(&deferred_config_queue, dev);
        config_process_deferred(&interrupt_config_queue, dev);
+       config_process_deferred(&mountroot_config_queue, dev);
 }
 
 static void
@@ -454,6 +458,30 @@
        }
 }
 
+static void
+config_mountroot_thread(void *cookie)
+{
+       struct deferred_config *dc;
+
+       while ((dc = TAILQ_FIRST(&mountroot_config_queue)) != NULL) {
+               TAILQ_REMOVE(&mountroot_config_queue, dc, dc_queue);
+               (*dc->dc_func)(dc->dc_dev);
+               kmem_free(dc, sizeof(*dc));
+       }
+       kthread_exit(0);
+}
+
+void
+config_create_mountrootthreads()
+{
+       int i;
+
+       for (i = 0; i < mountroot_config_threads; i++) {
+               (void)kthread_create(PRI_NONE, 0, NULL,
+                   config_mountroot_thread, NULL, NULL, "config");
+       }
+}
+
 /*
  * Announce device attach/detach to userland listeners.
  */
@@ -1848,6 +1876,39 @@
 }
 
 /*
+ * Defer some autoconfiguration for a device until after root file system
+ * is mounted (to load firmware etc).
+ */
+void
+config_mountroot(device_t dev, void (*func)(device_t))
+{
+       struct deferred_config *dc;
+
+       /*
+        * If root file system is mounted, callback now.
+        */
+       if (rootvnode != NULL) {
+               (*func)(dev);
+               return;
+       }
+
+#ifdef DIAGNOSTIC
+       TAILQ_FOREACH(dc, &mountroot_config_queue, dc_queue) {
+               if (dc->dc_dev == dev)
+                       panic("%s: deferred twice", __func__);
+       }
+#endif
+
+       dc = kmem_alloc(sizeof(*dc), KM_SLEEP);
+       if (dc == NULL)
+               panic("%s: unable to allocate callback", __func__);
+
+       dc->dc_dev = dev;
+       dc->dc_func = func;
+       TAILQ_INSERT_TAIL(&mountroot_config_queue, dc, dc_queue);
+}
+
+/*
  * Process a deferred configuration queue.
  */
 static void
Index: sys/device.h
===================================================================
RCS file: /cvsroot/src/sys/sys/device.h,v
retrieving revision 1.136
diff -u -r1.136 device.h
--- sys/device.h        25 Mar 2010 19:23:18 -0000      1.136
+++ sys/device.h        13 Jun 2010 03:00:15 -0000
@@ -479,9 +479,11 @@
 void   config_defer(device_t, void (*)(device_t));
 void   config_deferred(device_t);
 void   config_interrupts(device_t, void (*)(device_t));
+void   config_mountroot(device_t, void (*)(device_t));
 void   config_pending_incr(void);
 void   config_pending_decr(void);
 void   config_create_interruptthreads(void);
+void   config_create_mountrootthreads(void);
 
 int    config_finalize_register(device_t, int (*)(device_t));
 void   config_finalize(void);

---

upgt(4) (which requires firmware to get MAC address) on hpcarm
works fine with this patch.

(though I wonder if we should avoid such copy-n-pasted code..)

---
Izumi Tsutsui


Home | Main Index | Thread Index | Old Index