usb4bsd: Fixes, fixes, fixes.

* Fix a panic when trying to free null pointer in usb_free_device.

* Fix a panic due to wrong assignment of locks.

* Fix kqueue handling.

* Add debug helpers.

Submitted-by: Markus Pfeiffer <markus.pfeiffer@morphism.de>

diff --git a/sys/bus/u4b/usb_busdma.c b/sys/bus/u4b/usb_busdma.c
index 2fc9f21..c3c7136 100644 (file)
--- a/sys/bus/u4b/usb_busdma.c
+++ b/sys/bus/u4b/usb_busdma.c
@@ -372,7 +372,7 @@ usb_dma_tag_create(struct usb_dma_tag *udt,
            (2 + (size / USB_PAGE_SIZE)) : 1,
             /* maxsegsz  */ (align == 1 && size > USB_PAGE_SIZE) ?
            USB_PAGE_SIZE : size,
-            /* flags     */ BUS_DMA_ALIGNED | BUS_DMA_KEEP_PG_OFFSET, /* XXX: Find out what this is supposed to do! */
+            /* flags     */ BUS_DMA_KEEP_PG_OFFSET, /* XXX: Find out what this is supposed to do! */
            &tag)) {
                tag = NULL;
        }
@@ -458,7 +458,7 @@ usb_pc_common_mem_cb(void *arg, bus_dma_segment_t *segs,
        }
 
 done:
-       owned = (lockstatus(uptag->lock, curthread) == LK_EXCLUSIVE);
+       owned = lockowned(uptag->lock);
        if (!owned)
                lockmgr(uptag->lock, LK_EXCLUSIVE);
 
@@ -611,7 +611,7 @@ usb_pc_load_mem(struct usb_page_cache *pc, usb_size_t size, uint8_t sync)
        pc->page_offset_end = size;
        pc->ismultiseg = 1;
 
-       KKASSERT(lockstatus(pc->tag_parent->lock, curthread) != 0);
+       KKASSERT(lockowned(pc->tag_parent->lock));
 
        if (size > 0) {
                if (sync) {
@@ -869,7 +869,7 @@ usb_bdma_work_loop(struct usb_xfer_queue *pq)
        xfer = pq->curr;
        info = xfer->xroot;
 
-       KKASSERT(lockstatus(info->xfer_lock, curthread) != 0);
+       KKASSERT(lockowned(info->xfer_lock));
 
        if (xfer->error) {
                /* some error happened */
@@ -993,7 +993,7 @@ usb_bdma_done_event(struct usb_dma_parent_tag *udpt)
 
        info = USB_DMATAG_TO_XROOT(udpt);
 
-       KKASSERT(lockstatus(info->xfer_lock, curthread) != 0);
+       KKASSERT(lockowned(info->xfer_lock));
 
        /* copy error */
        info->dma_error = udpt->dma_error;

diff --git a/sys/bus/u4b/usb_core.h b/sys/bus/u4b/usb_core.h
index a011f19..3f2c69f 100644 (file)
--- a/sys/bus/u4b/usb_core.h
+++ b/sys/bus/u4b/usb_core.h
@@ -47,8 +47,8 @@
 #define        USB_BUS_LOCK_ASSERT_NOTOWNED(_b) KKASSERT(!lockowned(&(_b)->bus_lock))
 #define        USB_XFER_LOCK(_x)               lockmgr((_x)->xroot->xfer_lock, LK_EXCLUSIVE)
 #define        USB_XFER_UNLOCK(_x)             lockmgr((_x)->xroot->xfer_lock, LK_RELEASE)
-#define        USB_XFER_LOCK_ASSERT(_x)        KKASSERT(lockstatus((_x)->xroot->xfer_lock, curthread) != 0)
-#define        USB_XFER_LOCK_ASSERT_NOTOWNED(_x) KKASSERT(lockstatus((_x)->xroot->xfer_lock, curthread) == 0)
+#define        USB_XFER_LOCK_ASSERT(_x)        KKASSERT(lockowned((_x)->xroot->xfer_lock))
+#define        USB_XFER_LOCK_ASSERT_NOTOWNED(_x) KKASSERT(!lockowned((_x)->xroot->xfer_lock))
 
 /* helper for converting pointers to integers */
 #define        USB_P2U(ptr) \

diff --git a/sys/bus/u4b/usb_dev.c b/sys/bus/u4b/usb_dev.c
index 84b4076..8728ae3 100644 (file)
--- a/sys/bus/u4b/usb_dev.c
+++ b/sys/bus/u4b/usb_dev.c
@@ -33,6 +33,7 @@
 #include <sys/types.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
+#include <sys/thread2.h>
 #include <sys/bus.h>
 #include <sys/module.h>
 #include <sys/lock.h>
@@ -86,12 +87,7 @@ SYSCTL_INT(_hw_usb_dev, OID_AUTO, debug, CTLFLAG_RW,
 TUNABLE_INT("hw.usb.dev.debug", &usb_fifo_debug);
 #endif
 
-#if ((__FreeBSD_version >= 700001) || (__FreeBSD_version == 0) || \
-     ((__FreeBSD_version >= 600034) && (__FreeBSD_version < 700000)))
 #define        USB_UCRED struct ucred *ucred,
-#else
-#define        USB_UCRED
-#endif
 
 /* prototypes */
 
@@ -113,12 +109,18 @@ static usb_error_t usb_ref_device(struct usb_cdev_privdata *, struct usb_cdev_re
 static usb_error_t usb_usb_ref_device(struct usb_cdev_privdata *, struct usb_cdev_refdata *);
 static void    usb_unref_device(struct usb_cdev_privdata *, struct usb_cdev_refdata *);
 
+static void usb_filter_detach(struct knote *kn);
+static int usb_filter_read(struct knote *kn, long hint);
+static int usb_filter_write(struct knote *kn, long hint);
+
 static d_open_t usb_open;
 static d_close_t usb_close;
 static d_ioctl_t usb_ioctl;
 static d_read_t usb_read;
 static d_write_t usb_write;
 static d_kqfilter_t usb_kqfilter;
+static d_open_t usb_static_open;
+static d_close_t usb_static_close;
 static d_ioctl_t usb_static_ioctl;
 
 static usb_fifo_open_t usb_fifo_dummy_open;
@@ -142,6 +144,8 @@ static struct cdev* usb_dev = NULL;
 /* character device structure used for /bus/u4b */
 static struct dev_ops usb_static_ops = {
        { "usb", 0, D_MEM },
+       .d_open = usb_static_open,
+       .d_close = usb_static_close,
        .d_ioctl = usb_static_ioctl,
 };
 
@@ -150,6 +154,11 @@ static struct lock usb_sym_lock;
 
 struct lock usb_ref_lock;
 
+#if 0
+static int usb_nevents = 0;
+static struct kqinfo usb_kqevent;
+#endif
+
 /*------------------------------------------------------------------------*
  *     usb_loc_fill
  *
@@ -269,7 +278,7 @@ usb_ref_device(struct usb_cdev_privdata *cpd,
                DPRINTFN(2, "ref read\n");
                crd->rxfifo->refcount++;
        }
-       lockmgr(&usb_ref_lock, LK_EXCLUSIVE);
+       lockmgr(&usb_ref_lock, LK_RELEASE);
 
        return (0);
 
@@ -955,7 +964,7 @@ usb_dev_init(void *arg)
 }
 
 /* XXX SI_SUB_KLD? */
-SYSINIT(usb_dev_init, SI_SUB_CONFIGURE, SI_ORDER_FIRST, usb_dev_init, NULL);
+SYSINIT(usb_dev_init, SI_SUB_PRE_DRIVERS, SI_ORDER_FIRST, usb_dev_init, NULL);
 
 static void
 usb_dev_init_post(void *arg)
@@ -971,7 +980,8 @@ usb_dev_init_post(void *arg)
        }
 }
 
-SYSINIT(usb_dev_init_post, SI_SUB_KICK_SCHEDULER, SI_ORDER_FIRST, usb_dev_init_post, NULL);
+SYSINIT(usb_dev_init_post, SI_SUB_DRIVERS, SI_ORDER_FIRST, usb_dev_init_post,
+    NULL);
 
 static void
 usb_dev_uninit(void *arg)
@@ -1132,11 +1142,58 @@ done:
        return (err);
 }
 
+static struct filterops usb_filtops_read = 
+    { FILTEROP_ISFD, NULL, usb_filter_detach, usb_filter_read };
+
+static struct filterops usb_filtops_write = 
+    { FILTEROP_ISFD, NULL, usb_filter_detach, usb_filter_write };
+
 static int
 usb_kqfilter(struct dev_kqfilter_args *ap)
 {
-       usb_close(NULL);
-       return 0;
+       cdev_t dev = ap->a_head.a_dev;
+       struct knote *kn = ap->a_kn;
+
+       ap->a_result = 0;
+
+       switch(kn->kn_filter) {
+       case EVFILT_READ:
+               kn->kn_fop = &usb_filtops_read;
+               kn->kn_hook = (caddr_t)dev;
+               break;
+       case EVFILT_WRITE:
+               kn->kn_fop = &usb_filtops_write;
+               kn->kn_hook = (caddr_t)dev;
+               break;
+       default:
+               ap->a_result = EOPNOTSUPP;
+               return(0);
+       }
+
+       return(0);
+}
+
+static void
+usb_filter_detach(struct knote *kn)
+{
+#if 0
+       struct klist *klist;
+
+       klist = &usb_kqevent.ki_note; 
+       knote_remove(klist, kn);
+#endif
+}
+
+static int
+usb_filter_read(struct knote *kn, long hint)
+{
+       return(0);
+}
+
+static int
+usb_filter_write(struct knote *kn, long hint)
+{
+               return(0);
 }
 
 #if 0 /* XXX implement using kqfilter */
@@ -1531,16 +1588,25 @@ done:
        return (err);
 }
 
+
+static int
+usb_static_open(struct dev_open_args *ap)
+{
+       return(0);
+}
+
+static int
+usb_static_close(struct dev_close_args *ap)
+{
+       return(0);
+}
+
 int
 usb_static_ioctl(struct dev_ioctl_args *ap)
 {
        u_long cmd = ap->a_cmd;
        caddr_t data = ap->a_data;
-       /*
-        * XXX: what is this thread descriptor and where is it
-        * supposed to come from? 
-        */
-       struct thread *td = NULL;
+       struct thread *td = curthread; /* XXX: curthread the correct choice? */
        int fflag = ap->a_fflag;
        union {
                struct usb_read_dir *urd;
@@ -1601,7 +1667,7 @@ usb_fifo_wait(struct usb_fifo *f)
 {
        int err;
 
-       KKASSERT(lockstatus(f->priv_lock, curthread) != 0);
+       KKASSERT(lockowned(f->priv_lock));
 
        if (f->flag_iserror) {
                /* we are gone */

diff --git a/sys/bus/u4b/usb_device.c b/sys/bus/u4b/usb_device.c
index df91771..8713ed4 100644 (file)
--- a/sys/bus/u4b/usb_device.c
+++ b/sys/bus/u4b/usb_device.c
@@ -680,7 +680,7 @@ usb_config_parse(struct usb_device *udev, uint8_t iface_index, uint8_t cmd)
                goto cleanup;
 
        if (cmd == USB_CFG_INIT) {
-               KKASSERT(lockstatus(&udev->enum_lock, curthread) == LK_EXCLUSIVE);
+               KKASSERT(lockowned(&udev->enum_lock));
 
                /* check for in-use endpoints */
 
@@ -1467,13 +1467,13 @@ usbd_clear_stall_proc(struct usb_proc_msg *_pm)
 
        /* Change lock */
        USB_BUS_UNLOCK(udev->bus);
-       lockmgr(&udev->mtx_lock, LK_EXCLUSIVE);
+       lockmgr(&udev->device_lock, LK_EXCLUSIVE);
 
        /* Start clear stall callback */
        usbd_transfer_start(udev->ctrl_xfer[1]);
 
        /* Change lock */
-       lockmgr(&udev->mtx_lock, LK_RELEASE);
+       lockmgr(&udev->device_lock, LK_RELEASE);
        USB_BUS_LOCK(udev->bus);
 }
 
@@ -1541,14 +1541,11 @@ usb_alloc_device(device_t parent_dev, struct usb_bus *bus,
 #if 0
        /* initialise our SX-lock */
        sx_init_flags(&udev->ctrl_sx, "USB device SX lock", SX_DUPOK);
-#endif
-       lockinit(&udev->ctrl_lock, "USB device SX lock", 0, 0);
-
-#if 0
        /* initialise our SX-lock */
        sx_init_flags(&udev->enum_sx, "USB config SX lock", SX_DUPOK);
        sx_init_flags(&udev->sr_sx, "USB suspend and resume SX lock", SX_NOWITNESS);
 #endif
+       lockinit(&udev->ctrl_lock, "USB device SX lock", 0, 0);
        lockinit(&udev->enum_lock, "USB config SX lock", 0, 0);
        lockinit(&udev->sr_lock, "USB suspend and resume SX lock", 0, 0);
 
@@ -1556,7 +1553,7 @@ usb_alloc_device(device_t parent_dev, struct usb_bus *bus,
        cv_init(&udev->ref_cv, "UGONE");
 
        /* initialise our mutex */
-       lockinit(&udev->mtx_lock, "USB device mutex", 0, 0);
+       lockinit(&udev->device_lock, "USB device mutex", 0, 0);
 
        /* initialise generic clear stall */
        udev->cs_msg[0].hdr.pm_callback = &usbd_clear_stall_proc;
@@ -2120,7 +2117,7 @@ usb_free_device(struct usb_device *udev, uint8_t flag)
        cv_destroy(&udev->ctrlreq_cv);
        cv_destroy(&udev->ref_cv);
 
-       lockuninit(&udev->mtx_lock);
+       lockuninit(&udev->device_lock);
 #if USB_HAVE_UGEN
        KASSERT(LIST_FIRST(&udev->pd_list) == NULL, ("leaked cdev entries"));
 #endif
@@ -2130,9 +2127,12 @@ usb_free_device(struct usb_device *udev, uint8_t flag)
                (bus->methods->device_uninit) (udev);
 
        /* free device */
-       kfree(udev->serial, M_USB);
-       kfree(udev->manufacturer, M_USB);
-       kfree(udev->product, M_USB);
+       if(udev->serial)
+               kfree(udev->serial, M_USB);
+       if(udev->manufacturer)
+               kfree(udev->manufacturer, M_USB);
+       if(udev->product)
+               kfree(udev->product, M_USB);
        kfree(udev, M_USB);
 }
 
@@ -2715,7 +2715,7 @@ uint8_t
 usbd_enum_is_locked(struct usb_device *udev)
 {
        /* XXX: Make sure that we return a correct value here */
-       return (lockstatus(&udev->enum_lock, curthread) == LK_EXCLUSIVE);
+       return (lockowned(&udev->enum_lock));
 }
 
 /*

diff --git a/sys/bus/u4b/usb_device.h b/sys/bus/u4b/usb_device.h
index e9872bc..8c1ea4e 100644 (file)
--- a/sys/bus/u4b/usb_device.h
+++ b/sys/bus/u4b/usb_device.h
@@ -117,7 +117,6 @@ struct usb_device {
        struct lock ctrl_lock;
        struct lock enum_lock;
        struct lock sr_lock;
-       struct lock mtx_lock;
        struct lock device_lock;
        struct cv ctrlreq_cv;
        struct cv ref_cv;

diff --git a/sys/bus/u4b/usb_generic.c b/sys/bus/u4b/usb_generic.c
index 5b3e052..b585e42 100644 (file)
--- a/sys/bus/u4b/usb_generic.c
+++ b/sys/bus/u4b/usb_generic.c
@@ -232,7 +232,7 @@ ugen_open_pipe_write(struct usb_fifo *f)
        struct usb_endpoint *ep = usb_fifo_softc(f);
        struct usb_endpoint_descriptor *ed = ep->edesc;
 
-       KKASSERT(lockstatus(f->priv_lock, curthread) != 0);
+       KKASSERT(lockowned(f->priv_lock));
 
        if (f->xfer[0] || f->xfer[1]) {
                /* transfers are already opened */
@@ -300,7 +300,7 @@ ugen_open_pipe_read(struct usb_fifo *f)
        struct usb_endpoint *ep = usb_fifo_softc(f);
        struct usb_endpoint_descriptor *ed = ep->edesc;
 
-       KKASSERT(lockstatus(f->priv_lock, curthread) != 0);
+       KKASSERT(lockowned(f->priv_lock));
 
        if (f->xfer[0] || f->xfer[1]) {
                /* transfers are already opened */

diff --git a/sys/bus/u4b/usb_hub.c b/sys/bus/u4b/usb_hub.c
index 7b08a35..54fdcc0 100644 (file)
--- a/sys/bus/u4b/usb_hub.c
+++ b/sys/bus/u4b/usb_hub.c
@@ -1923,7 +1923,7 @@ usb_needs_explore(struct usb_bus *bus, uint8_t do_probe)
                DPRINTF("No root HUB\n");
                return;
        }
-       if (lockstatus(&bus->bus_lock, curthread) != 0) {
+       if (lockowned(&bus->bus_lock) != 0) {
                do_unlock = 0;
        } else {
                USB_BUS_LOCK(bus);

diff --git a/sys/bus/u4b/usb_process.c b/sys/bus/u4b/usb_process.c
index 2ca2d16..5093f42 100644 (file)
--- a/sys/bus/u4b/usb_process.c
+++ b/sys/bus/u4b/usb_process.c
@@ -269,7 +269,7 @@ usb_proc_msignal(struct usb_process *up, void *_pm0, void *_pm1)
        if (up->up_gone)
                return (_pm0);
 
-       KKASSERT(lockstatus(up->up_lock, curthread) != 0);
+       KKASSERT(lockowned(up->up_lock));
 
        t = 0;
 
@@ -350,7 +350,7 @@ usb_proc_is_gone(struct usb_process *up)
         * structure is initialised.
         */
        if (up->up_lock != NULL)
-               KKASSERT(lockstatus(up->up_lock, curthread)!=0);
+               KKASSERT(lockowned(up->up_lock));
        return (0);
 }
 
@@ -371,7 +371,7 @@ usb_proc_mwait(struct usb_process *up, void *_pm0, void *_pm1)
        if (up->up_gone)
                return;
 
-       KKASSERT(lockstatus(up->up_lock, curthread) != 0);
+       KKASSERT(lockowned(up->up_lock));
 
        if (up->up_curtd == curthread) {
                /* Just remove the messages from the queue. */
@@ -414,7 +414,7 @@ usb_proc_drain(struct usb_process *up)
        if (up->up_mtx != &Giant)
                mtx_assert(up->up_mtx, MA_NOTOWNED);
 #else
-       KKASSERT(lockstatus(up->up_lock, curthread) == 0);
+       KKASSERT(!lockowned(up->up_lock));
        lockmgr(up->up_lock, LK_EXCLUSIVE);
 #endif
 
@@ -470,7 +470,7 @@ usb_proc_rewakeup(struct usb_process *up)
        if (up->up_gone)
                return;
 
-       KKASSERT(lockstatus(up->up_lock, curthread) != 0);
+       KKASSERT(lockowned(up->up_lock));
 
        if (up->up_msleep == 0) {
                /* re-wakeup */

diff --git a/sys/bus/u4b/usb_request.c b/sys/bus/u4b/usb_request.c
index 352053b..c8c43b3 100644 (file)
--- a/sys/bus/u4b/usb_request.c
+++ b/sys/bus/u4b/usb_request.c
@@ -457,7 +457,7 @@ usbd_do_request_flags(struct usb_device *udev, struct lock *lock,
 #endif
        if (lock != NULL) {
                lockmgr(lock, LK_RELEASE);
-               KKASSERT(lockstatus(lock, curthread) == 0);
+               KKASSERT(!lockowned(lock));
        }
 
        /*

diff --git a/sys/bus/u4b/usb_transfer.c b/sys/bus/u4b/usb_transfer.c
index 6c05dfe..cc2be18 100644 (file)
--- a/sys/bus/u4b/usb_transfer.c
+++ b/sys/bus/u4b/usb_transfer.c
@@ -841,6 +841,7 @@ usbd_transfer_setup(struct usb_device *udev,
                return (USB_ERR_INVAL);
        }
        if (xfer_lock == NULL) {
+               panic("xfer without lock!\n");
                DPRINTFN(6, "using global lock\n");
        }
        /* sanity checks */
@@ -1675,9 +1676,7 @@ usbd_pipe_enter(struct usb_xfer *xfer)
 {
        struct usb_endpoint *ep;
 
-#if 0
-       USB_XFER_LOCK_ASSERT(xfer, MA_OWNED);
-#endif
+       USB_XFER_LOCK_ASSERT(xfer);
 
        USB_BUS_LOCK(xfer->xroot->bus);
 
@@ -1717,9 +1716,7 @@ usbd_transfer_start(struct usb_xfer *xfer)
                /* transfer is gone */
                return;
        }
-#if 0
-       USB_XFER_LOCK_ASSERT(xfer, MA_OWNED);
-#endif
+       USB_XFER_LOCK_ASSERT(xfer);
 
        /* mark the USB transfer started */
 
@@ -2161,10 +2158,7 @@ usbd_callback_wrapper(struct usb_xfer_queue *pq)
        struct usb_xfer_root *info = xfer->xroot;
 
        USB_BUS_LOCK_ASSERT(info->bus);
-#if 0 /* XXX: This is probably to prevent deadlocks */
-       if (!lockstatus(info->xfer_mtx, curthread) && !SCHEDULER_STOPPED()) {
-#endif
-       if (!lockstatus(info->xfer_lock, curthread)) {
+       if (!lockowned(info->xfer_lock)) {
                /*
                 * Cases that end up here:
                 *
@@ -2241,6 +2235,7 @@ usbd_callback_wrapper(struct usb_xfer_queue *pq)
        if (xfer->usb_state != USB_ST_SETUP)
                usbpf_xfertap(xfer, USBPF_XFERTAP_DONE);
 #endif
+       USB_XFER_LOCK_ASSERT(xfer);
        /* call processing routine */
        (xfer->callback) (xfer, xfer->error);
 
@@ -2383,7 +2378,7 @@ usbd_transfer_done(struct usb_xfer *xfer, usb_error_t error)
        usbd_transfer_dequeue(xfer);
 
 #if USB_HAVE_BUSDMA
-       if (lockstatus(xfer->xroot->xfer_lock, curthread)) {
+       if (lockowned(xfer->xroot->xfer_lock)) {
                struct usb_xfer_queue *pq;
 
                /*
@@ -3133,20 +3128,14 @@ usbd_transfer_poll(struct usb_xfer **ppxfer, uint16_t max)
 
                /* make sure that the BUS mutex is not locked */
                drop_bus = 0;
-#if 0 /* XXX */
-               while (lockstatus(&xroot->udev->bus->bus_lock, curthread) && !SCHEDULER_STOPPED()) {
-#endif
-               while (lockstatus(&xroot->udev->bus->bus_lock, curthread)) {
+               while (lockowned(&xroot->udev->bus->bus_lock)) {
                        lockmgr(&xroot->udev->bus->bus_lock, LK_RELEASE);
                        drop_bus++;
                }
 
                /* make sure that the transfer mutex is not locked */
                drop_xfer = 0;
-#if 0 /* XXX */
-               while (lockstatus(xroot->xfer_lock, curthread) && !SCHEDULER_STOPPED()) {
-#endif
-               while (lockstatus(xroot->xfer_lock, curthread)) {
+               while (lockowned(xroot->xfer_lock)) {
                        lockmgr(xroot->xfer_lock, LK_RELEASE);
                        drop_xfer++;
                }

diff --git a/sys/bus/u4b/usbdi.h b/sys/bus/u4b/usbdi.h
index e5b3926..451eaba 100644 (file)
--- a/sys/bus/u4b/usbdi.h
+++ b/sys/bus/u4b/usbdi.h
@@ -393,7 +393,6 @@ struct usb_callout {
        void *uco_arg;
        int uco_flags;
 };
-/* XXX what is supposed to happen with that mutex? */
 
 void usb_callout_timeout_wrapper(void *arg);
 void usb_callout_init_mtx_dfly(struct usb_callout *uco, struct lock *lock,