Date: Thu, 12 Mar 2009 02:51:55 +0000 (UTC) From: Weongyo Jeong <weongyo@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r189719 - in head/sys: compat/ndis dev/if_ndis Message-ID: <200903120251.n2C2pt04014409@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: weongyo Date: Thu Mar 12 02:51:55 2009 New Revision: 189719 URL: http://svn.freebsd.org/changeset/base/189719 Log: o change a lock model based on HAL preemption lock to a normal mtx. Based on the HAL preemption lock there is a problem on SMP machines and causes a panic. o When a device detached the current tactic to detach NDIS USB driver is to call SURPRISE_REMOVED event. So it don't need to call ndis_halt_nic() again. This fixes some page faults when some drivers work abnormal. o it assumes now that URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER is in DISPATCH_LEVEL (non-sleepable) and as further work URB_FUNCTION_VENDOR_XXX and URB_FUNCTION_CLASS_XXX should be. Reviewed by: Hans Petter Selasky <hselasky_at_freebsd.org> Tested by: Paul B. Mahol <onemda_at_gmail.com> Modified: head/sys/compat/ndis/hal_var.h head/sys/compat/ndis/subr_hal.c head/sys/compat/ndis/subr_usbd.c head/sys/dev/if_ndis/if_ndis.c head/sys/dev/if_ndis/if_ndis_usb.c head/sys/dev/if_ndis/if_ndisvar.h Modified: head/sys/compat/ndis/hal_var.h ============================================================================== --- head/sys/compat/ndis/hal_var.h Thu Mar 12 02:32:54 2009 (r189718) +++ head/sys/compat/ndis/hal_var.h Thu Mar 12 02:51:55 2009 (r189719) @@ -48,7 +48,6 @@ extern image_patch_table hal_functbl[]; __BEGIN_DECLS extern int hal_libinit(void); extern int hal_libfini(void); -extern struct mtx *hal_getdisplock(void); extern uint8_t KfAcquireSpinLock(kspin_lock *); extern void KfReleaseSpinLock(kspin_lock *, uint8_t); extern uint8_t KfRaiseIrql(uint8_t); Modified: head/sys/compat/ndis/subr_hal.c ============================================================================== --- head/sys/compat/ndis/subr_hal.c Thu Mar 12 02:32:54 2009 (r189718) +++ head/sys/compat/ndis/subr_hal.c Thu Mar 12 02:51:55 2009 (r189719) @@ -124,13 +124,6 @@ hal_libfini() return(0); } -struct mtx * -hal_getdisplock() -{ - - return &disp_lock[curthread->td_oncpu]; -} - static void KeStallExecutionProcessor(usecs) uint32_t usecs; Modified: head/sys/compat/ndis/subr_usbd.c ============================================================================== --- head/sys/compat/ndis/subr_usbd.c Thu Mar 12 02:32:54 2009 (r189718) +++ head/sys/compat/ndis/subr_usbd.c Thu Mar 12 02:51:55 2009 (r189719) @@ -93,6 +93,8 @@ static int32_t usbd_power(device_objec static void usbd_irpcancel(device_object *, irp *); static int32_t usbd_submit_urb(irp *); static int32_t usbd_urb2nt(int32_t); +static void usbd_task(device_object *, void *); +static int32_t usbd_taskadd(irp *, unsigned); static void usbd_xfertask(device_object *, void *); static void dummy(void); @@ -118,6 +120,7 @@ static funcptr usbd_ioinvalid_wrap; static funcptr usbd_pnp_wrap; static funcptr usbd_power_wrap; static funcptr usbd_irpcancel_wrap; +static funcptr usbd_task_wrap; static funcptr usbd_xfertask_wrap; int @@ -144,6 +147,8 @@ usbd_libinit(void) (funcptr *)&usbd_power_wrap, 2, WINDRV_WRAP_STDCALL); windrv_wrap((funcptr)usbd_irpcancel, (funcptr *)&usbd_irpcancel_wrap, 2, WINDRV_WRAP_STDCALL); + windrv_wrap((funcptr)usbd_task, + (funcptr *)&usbd_task_wrap, 2, WINDRV_WRAP_STDCALL); windrv_wrap((funcptr)usbd_xfertask, (funcptr *)&usbd_xfertask_wrap, 2, WINDRV_WRAP_STDCALL); @@ -184,6 +189,7 @@ usbd_libfini(void) windrv_unwrap(usbd_pnp_wrap); windrv_unwrap(usbd_power_wrap); windrv_unwrap(usbd_irpcancel_wrap); + windrv_unwrap(usbd_task_wrap); windrv_unwrap(usbd_xfertask_wrap); free(usbd_driver.dro_drivername.us_buf, M_DEVBUF); @@ -417,7 +423,6 @@ usbd_func_getdesc(ip) device_t dev = IRP_NDIS_DEV(ip); struct ndis_softc *sc = device_get_softc(dev); struct usbd_urb_control_descriptor_request *ctldesc; - uint8_t irql; uint16_t actlen; uint32_t len; union usbd_urb *urb; @@ -450,13 +455,13 @@ usbd_func_getdesc(ip) actlen = len; status = USB_ERR_NORMAL_COMPLETION; } else { - KeRaiseIrql(DISPATCH_LEVEL, &irql); - status = usb2_req_get_desc(sc->ndisusb_dev, hal_getdisplock(), + NDISUSB_LOCK(sc); + status = usb2_req_get_desc(sc->ndisusb_dev, &sc->ndisusb_mtx, &actlen, ctldesc->ucd_trans_buf, 2, ctldesc->ucd_trans_buflen, ctldesc->ucd_langid, ctldesc->ucd_desctype, ctldesc->ucd_idx, NDISUSB_GETDESC_MAXRETRIES); - KeLowerIrql(irql); + NDISUSB_UNLOCK(sc); } exit: if (status != USB_ERR_NORMAL_COMPLETION) { @@ -497,11 +502,6 @@ usbd_func_selconf(ip) return usbd_usb2urb(USB_ERR_NORMAL_COMPLETION); } - if (conf->bConfigurationValue > NDISUSB_CONFIG_NO) - device_printf(dev, - "warning: config_no is larger than default (%#x/%#x)\n", - conf->bConfigurationValue, NDISUSB_CONFIG_NO); - intf = &selconf->usc_intf; for (i = 0; i < conf->bNumInterface && intf->uii_len > 0; i++) { ret = usb2_set_alt_interface_index(udev, @@ -594,7 +594,7 @@ usbd_setup_endpoint(ip, ifidx, ep) cfg.mh.flags.short_xfer_ok = 1; status = usb2_transfer_setup(sc->ndisusb_dev, &ifidx, ne->ne_xfer, - &cfg, 1, sc, hal_getdisplock()); + &cfg, 1, sc, &sc->ndisusb_mtx); if (status != USB_ERR_NORMAL_COMPLETION) { device_printf(dev, "couldn't setup xfer: %s\n", usb2_errstr(status)); @@ -618,8 +618,9 @@ static int32_t usbd_func_abort_pipe(ip) irp *ip; { + device_t dev = IRP_NDIS_DEV(ip); + struct ndis_softc *sc = device_get_softc(dev); struct ndisusb_ep *ne; - uint8_t irql; union usbd_urb *urb; urb = usbd_geturb(ip); @@ -629,10 +630,10 @@ usbd_func_abort_pipe(ip) return (USBD_STATUS_INVALID_PIPE_HANDLE); } - KeRaiseIrql(DISPATCH_LEVEL, &irql); + NDISUSB_LOCK(sc); usb2_transfer_stop(ne->ne_xfer[0]); usb2_transfer_start(ne->ne_xfer[0]); - KeLowerIrql(irql); + NDISUSB_UNLOCK(sc); return (USBD_STATUS_SUCCESS); } @@ -644,7 +645,7 @@ usbd_func_vendorclass(ip) device_t dev = IRP_NDIS_DEV(ip); struct ndis_softc *sc = device_get_softc(dev); struct usbd_urb_vendor_or_class_request *vcreq; - uint8_t irql, type = 0; + uint8_t type = 0; union usbd_urb *urb; struct usb2_device_request req; usb2_error_t status; @@ -692,10 +693,10 @@ usbd_func_vendorclass(ip) USETW(req.wValue, vcreq->uvc_value); USETW(req.wLength, vcreq->uvc_trans_buflen); - KeRaiseIrql(DISPATCH_LEVEL, &irql); - status = usb2_do_request(sc->ndisusb_dev, hal_getdisplock(), &req, + NDISUSB_LOCK(sc); + status = usb2_do_request(sc->ndisusb_dev, &sc->ndisusb_mtx, &req, vcreq->uvc_trans_buf); - KeLowerIrql(irql); + NDISUSB_UNLOCK(sc); return usbd_usb2urb(status); } @@ -755,19 +756,18 @@ static struct ndisusb_xfer * usbd_aq_getfirst(struct ndis_softc *sc, struct ndisusb_ep *ne) { struct ndisusb_xfer *nx; - uint8_t irql; - KeAcquireSpinLock(&ne->ne_lock, &irql); + KeAcquireSpinLockAtDpcLevel(&ne->ne_lock); if (IsListEmpty(&ne->ne_active)) { device_printf(sc->ndis_dev, "%s: the active queue can't be empty.\n", __func__); - KeReleaseSpinLock(&ne->ne_lock, irql); + KeReleaseSpinLockFromDpcLevel(&ne->ne_lock); return (NULL); } nx = CONTAINING_RECORD(ne->ne_active.nle_flink, struct ndisusb_xfer, nx_next); RemoveEntryList(&nx->nx_next); - KeReleaseSpinLock(&ne->ne_lock, irql); + KeReleaseSpinLockFromDpcLevel(&ne->ne_lock); return (nx); } @@ -881,10 +881,8 @@ usbd_get_ndisep(ip, ep) ne = &sc->ndisusb_ep[NDISUSB_GET_ENDPT(ep->bEndpointAddress)]; - IoAcquireCancelSpinLock(&ip->irp_cancelirql); IRP_NDISUSB_EP(ip) = ne; ip->irp_cancelfunc = (cancel_func)usbd_irpcancel_wrap; - IoReleaseCancelSpinLock(ip->irp_cancelirql); return (ne); } @@ -902,7 +900,6 @@ usbd_xfertask(dobj, arg) struct ndisusb_xferdone *nd; struct ndisusb_xfer *nq; struct usbd_urb_bulk_or_intr_transfer *ubi; - uint8_t irql; union usbd_urb *urb; usb2_error_t status; void *priv; @@ -912,7 +909,7 @@ usbd_xfertask(dobj, arg) if (IsListEmpty(&sc->ndisusb_xferdonelist)) return; - KeAcquireSpinLock(&sc->ndisusb_xferdonelock, &irql); + KeAcquireSpinLockAtDpcLevel(&sc->ndisusb_xferdonelock); l = sc->ndisusb_xferdonelist.nle_flink; while (l != &sc->ndisusb_xferdonelist) { nd = CONTAINING_RECORD(l, struct ndisusb_xferdone, nd_donelist); @@ -928,8 +925,6 @@ usbd_xfertask(dobj, arg) ("function(%d) isn't for bulk or interrupt", urb->uu_hdr.uuh_func)); - IoAcquireCancelSpinLock(&ip->irp_cancelirql); - ip->irp_cancelfunc = NULL; IRP_NDISUSB_EP(ip) = NULL; @@ -954,28 +949,114 @@ usbd_xfertask(dobj, arg) break; } - IoReleaseCancelSpinLock(ip->irp_cancelirql); - l = l->nle_flink; RemoveEntryList(&nd->nd_donelist); free(nq, M_USBDEV); free(nd, M_USBDEV); if (error) continue; + KeReleaseSpinLockFromDpcLevel(&sc->ndisusb_xferdonelock); /* NB: call after cleaning */ IoCompleteRequest(ip, IO_NO_INCREMENT); + KeAcquireSpinLockAtDpcLevel(&sc->ndisusb_xferdonelock); } - KeReleaseSpinLock(&sc->ndisusb_xferdonelock, irql); + KeReleaseSpinLockFromDpcLevel(&sc->ndisusb_xferdonelock); +} + +/* + * this function is for mainly deferring a task to the another thread because + * we don't want to be in the scope of HAL lock. + */ +static int32_t +usbd_taskadd(ip, type) + irp *ip; + unsigned type; +{ + device_t dev = IRP_NDIS_DEV(ip); + struct ndis_softc *sc = device_get_softc(dev); + struct ndisusb_task *nt; + + nt = malloc(sizeof(struct ndisusb_task), M_USBDEV, M_NOWAIT | M_ZERO); + if (nt == NULL) + return (USBD_STATUS_NO_MEMORY); + nt->nt_type = type; + nt->nt_ctx = ip; + + KeAcquireSpinLockAtDpcLevel(&sc->ndisusb_tasklock); + InsertTailList((&sc->ndisusb_tasklist), (&nt->nt_tasklist)); + KeReleaseSpinLockFromDpcLevel(&sc->ndisusb_tasklock); + + IoQueueWorkItem(sc->ndisusb_taskitem, + (io_workitem_func)usbd_task_wrap, WORKQUEUE_CRITICAL, sc); + + return (USBD_STATUS_SUCCESS); +} + +static void +usbd_task(dobj, arg) + device_object *dobj; + void *arg; +{ + irp *ip; + list_entry *l; + struct ndis_softc *sc = arg; + struct ndisusb_ep *ne; + struct ndisusb_task *nt; + union usbd_urb *urb; + + if (IsListEmpty(&sc->ndisusb_tasklist)) + return; + + KeAcquireSpinLockAtDpcLevel(&sc->ndisusb_tasklock); + l = sc->ndisusb_tasklist.nle_flink; + while (l != &sc->ndisusb_tasklist) { + nt = CONTAINING_RECORD(l, struct ndisusb_task, nt_tasklist); + + ip = nt->nt_ctx; + urb = usbd_geturb(ip); + + KeReleaseSpinLockFromDpcLevel(&sc->ndisusb_tasklock); + NDISUSB_LOCK(sc); + switch (nt->nt_type) { + case NDISUSB_TASK_TSTART: + ne = usbd_get_ndisep(ip, urb->uu_bulkintr.ubi_epdesc); + if (ne == NULL) + goto exit; + usb2_transfer_start(ne->ne_xfer[0]); + break; + case NDISUSB_TASK_IRPCANCEL: + ne = usbd_get_ndisep(ip, + (nt->nt_type == NDISUSB_TASK_IRPCANCEL) ? + urb->uu_bulkintr.ubi_epdesc : + urb->uu_pipe.upr_handle); + if (ne == NULL) + goto exit; + + usb2_transfer_stop(ne->ne_xfer[0]); + usb2_transfer_start(ne->ne_xfer[0]); + break; + default: + break; + } +exit: + NDISUSB_UNLOCK(sc); + KeAcquireSpinLockAtDpcLevel(&sc->ndisusb_tasklock); + + l = l->nle_flink; + RemoveEntryList(&nt->nt_tasklist); + free(nt, M_USBDEV); + } + KeReleaseSpinLockFromDpcLevel(&sc->ndisusb_tasklock); } static int32_t usbd_func_bulkintr(ip) irp *ip; { + int32_t error; struct ndisusb_ep *ne; struct ndisusb_xfer *nx; struct usbd_urb_bulk_or_intr_transfer *ubi; - uint8_t irql; union usbd_urb *urb; usb_endpoint_descriptor_t *ep; @@ -998,9 +1079,9 @@ usbd_func_bulkintr(ip) } nx->nx_ep = ne; nx->nx_priv = ip; - KeAcquireSpinLock(&ne->ne_lock, &irql); + KeAcquireSpinLockAtDpcLevel(&ne->ne_lock); InsertTailList((&ne->ne_pending), (&nx->nx_next)); - KeReleaseSpinLock(&ne->ne_lock, irql); + KeReleaseSpinLockFromDpcLevel(&ne->ne_lock); /* we've done to setup xfer. Let's transfer it. */ ip->irp_iostat.isb_status = STATUS_PENDING; @@ -1008,9 +1089,9 @@ usbd_func_bulkintr(ip) USBD_URB_STATUS(urb) = USBD_STATUS_PENDING; IoMarkIrpPending(ip); - KeRaiseIrql(DISPATCH_LEVEL, &irql); - usb2_transfer_start(ne->ne_xfer[0]); - KeLowerIrql(irql); + error = usbd_taskadd(ip, NDISUSB_TASK_TSTART); + if (error != USBD_STATUS_SUCCESS) + return (error); return (USBD_STATUS_PENDING); } Modified: head/sys/dev/if_ndis/if_ndis.c ============================================================================== --- head/sys/dev/if_ndis/if_ndis.c Thu Mar 12 02:32:54 2009 (r189718) +++ head/sys/dev/if_ndis/if_ndis.c Thu Mar 12 02:51:55 2009 (r189719) @@ -557,8 +557,10 @@ ndis_attach(dev) mtx_init(&sc->ndis_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK, MTX_DEF); KeInitializeSpinLock(&sc->ndis_rxlock); + KeInitializeSpinLock(&sc->ndisusb_tasklock); KeInitializeSpinLock(&sc->ndisusb_xferdonelock); InitializeListHead(&sc->ndis_shlist); + InitializeListHead(&sc->ndisusb_tasklist); InitializeListHead(&sc->ndisusb_xferdonelist); callout_init(&sc->ndis_stat_callout, CALLOUT_MPSAFE); @@ -625,6 +627,8 @@ ndis_attach(dev) sc->ndis_inputitem = IoAllocateWorkItem(sc->ndis_block->nmb_deviceobj); sc->ndisusb_xferdoneitem = IoAllocateWorkItem(sc->ndis_block->nmb_deviceobj); + sc->ndisusb_taskitem = + IoAllocateWorkItem(sc->ndis_block->nmb_deviceobj); KeInitializeDpc(&sc->ndis_rxdpc, ndis_rxeof_xfr_wrap, sc->ndis_block); /* Call driver's init routine. */ @@ -1066,6 +1070,8 @@ ndis_detach(dev) IoFreeWorkItem(sc->ndis_inputitem); if (sc->ndisusb_xferdoneitem != NULL) IoFreeWorkItem(sc->ndisusb_xferdoneitem); + if (sc->ndisusb_taskitem != NULL) + IoFreeWorkItem(sc->ndisusb_taskitem); bus_generic_detach(dev); ndis_unload_driver(sc); @@ -3236,8 +3242,10 @@ ndis_stop(sc) ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE); NDIS_UNLOCK(sc); - if (!(sc->ndis_iftype == PNPBus && ndisusb_halt == 0) || - sc->ndisusb_status & NDISUSB_STATUS_DETACH) + if (sc->ndis_iftype != PNPBus || + (sc->ndis_iftype == PNPBus && + !(sc->ndisusb_status & NDISUSB_STATUS_DETACH) && + ndisusb_halt != 0)) ndis_halt_nic(sc); NDIS_LOCK(sc); Modified: head/sys/dev/if_ndis/if_ndis_usb.c ============================================================================== --- head/sys/dev/if_ndis/if_ndis_usb.c Thu Mar 12 02:32:54 2009 (r189718) +++ head/sys/dev/if_ndis/if_ndis_usb.c Thu Mar 12 02:51:55 2009 (r189719) @@ -168,6 +168,7 @@ ndisusb_attach(device_t self) db = uaa->driver_info; sc = (struct ndis_softc *)dummy; sc->ndis_dev = self; + mtx_init(&sc->ndisusb_mtx, "NDIS USB", MTX_NETWORK_LOCK, MTX_DEF); sc->ndis_dobj = db->windrv_object; sc->ndis_regvals = db->windrv_regvals; sc->ndis_iftype = PNPBus; @@ -207,14 +208,17 @@ ndisusb_detach(device_t self) sc->ndisusb_status |= NDISUSB_STATUS_DETACH; + ndis_pnpevent_nic(self, NDIS_PNP_EVENT_SURPRISE_REMOVED); + for (i = 0; i < NDISUSB_ENDPT_MAX; i++) { ne = &sc->ndisusb_ep[i]; usb2_transfer_unsetup(ne->ne_xfer, 1); } - ndis_pnpevent_nic(self, NDIS_PNP_EVENT_SURPRISE_REMOVED); + (void)ndis_detach(self); - return ndis_detach(self); + mtx_destroy(&sc->ndisusb_mtx); + return (0); } static struct resource_list * Modified: head/sys/dev/if_ndis/if_ndisvar.h ============================================================================== --- head/sys/dev/if_ndis/if_ndisvar.h Thu Mar 12 02:32:54 2009 (r189718) +++ head/sys/dev/if_ndis/if_ndisvar.h Thu Mar 12 02:51:55 2009 (r189719) @@ -142,6 +142,14 @@ struct ndisusb_xferdone { list_entry nd_donelist; }; +struct ndisusb_task { + unsigned nt_type; +#define NDISUSB_TASK_TSTART 0 +#define NDISUSB_TASK_IRPCANCEL 1 + void *nt_ctx; + list_entry nt_tasklist; +}; + struct ndis_softc { struct ifnet *ifp; struct ifmedia ifmedia; /* media info */ @@ -220,6 +228,7 @@ struct ndis_softc { int ndis_hang_timer; struct usb2_device *ndisusb_dev; + struct mtx ndisusb_mtx; #define NDISUSB_GET_ENDPT(addr) \ ((UE_GET_DIR(addr) >> 7) | (UE_GET_ADDR(addr) << 1)) #define NDISUSB_ENDPT_MAX ((UE_ADDR + 1) * 2) @@ -227,6 +236,9 @@ struct ndis_softc { io_workitem *ndisusb_xferdoneitem; list_entry ndisusb_xferdonelist; kspin_lock ndisusb_xferdonelock; + io_workitem *ndisusb_taskitem; + list_entry ndisusb_tasklist; + kspin_lock ndisusb_tasklock; int ndisusb_status; #define NDISUSB_STATUS_DETACH 0x1 }; @@ -234,3 +246,7 @@ struct ndis_softc { #define NDIS_LOCK(_sc) mtx_lock(&(_sc)->ndis_mtx) #define NDIS_UNLOCK(_sc) mtx_unlock(&(_sc)->ndis_mtx) #define NDIS_LOCK_ASSERT(_sc, t) mtx_assert(&(_sc)->ndis_mtx, t) +#define NDISUSB_LOCK(_sc) mtx_lock(&(_sc)->ndisusb_mtx) +#define NDISUSB_UNLOCK(_sc) mtx_unlock(&(_sc)->ndisusb_mtx) +#define NDISUSB_LOCK_ASSERT(_sc, t) mtx_assert(&(_sc)->ndisusb_mtx, t) +
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200903120251.n2C2pt04014409>