Module Name: src Committed By: martin Date: Sun Mar 10 18:36:12 UTC 2024
Modified Files: src/sys/dev/usb [netbsd-10]: usbdi.c Log Message: Pull up following revision(s) (requested by riastradh in ticket #613): sys/dev/usb/usbdi.c: revision 1.248 sys/dev/usb/usbdi.c: revision 1.249 usbdi(9): Avoid calling ubm_softint with lock held and polling on. usbdi(9): Avoid taking locks in usbd_transfer while polling. PR kern/57783 To generate a diff of this commit: cvs rdiff -u -r1.247 -r1.247.4.1 src/sys/dev/usb/usbdi.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/dev/usb/usbdi.c diff -u src/sys/dev/usb/usbdi.c:1.247 src/sys/dev/usb/usbdi.c:1.247.4.1 --- src/sys/dev/usb/usbdi.c:1.247 Tue Sep 13 10:32:58 2022 +++ src/sys/dev/usb/usbdi.c Sun Mar 10 18:36:12 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: usbdi.c,v 1.247 2022/09/13 10:32:58 riastradh Exp $ */ +/* $NetBSD: usbdi.c,v 1.247.4.1 2024/03/10 18:36:12 martin Exp $ */ /* * Copyright (c) 1998, 2012, 2015 The NetBSD Foundation, Inc. @@ -32,7 +32,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.247 2022/09/13 10:32:58 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.247.4.1 2024/03/10 18:36:12 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -410,14 +410,18 @@ usbd_transfer(struct usbd_xfer *xfer) } } - usbd_lock_pipe(pipe); + if (pipe->up_dev->ud_bus->ub_usepolling == 0) + usbd_lock_pipe(pipe); if (pipe->up_aborting) { /* * XXX For synchronous transfers this is fine. What to * do for asynchronous transfers? The callback is * never run, not even with status USBD_CANCELLED. + * + * XXX Does it make sense to abort while polling? */ - usbd_unlock_pipe(pipe); + if (pipe->up_dev->ud_bus->ub_usepolling == 0) + usbd_unlock_pipe(pipe); USBHIST_LOG(usbdebug, "<- done xfer %#jx, aborting", (uintptr_t)xfer, 0, 0, 0); SDT_PROBE2(usb, device, xfer, done, xfer, USBD_CANCELLED); @@ -443,7 +447,8 @@ usbd_transfer(struct usbd_xfer *xfer) } while (0); SDT_PROBE3(usb, device, pipe, transfer__done, pipe, xfer, err); - usbd_unlock_pipe(pipe); + if (pipe->up_dev->ud_bus->ub_usepolling == 0) + usbd_unlock_pipe(pipe); if (err != USBD_IN_PROGRESS && err) { /* @@ -453,7 +458,8 @@ usbd_transfer(struct usbd_xfer *xfer) */ USBHIST_LOG(usbdebug, "xfer failed: %jd, reinserting", err, 0, 0, 0); - usbd_lock_pipe(pipe); + if (pipe->up_dev->ud_bus->ub_usepolling == 0) + usbd_lock_pipe(pipe); SDT_PROBE1(usb, device, xfer, preabort, xfer); #ifdef DIAGNOSTIC xfer->ux_state = XFER_BUSY; @@ -461,7 +467,8 @@ usbd_transfer(struct usbd_xfer *xfer) SIMPLEQ_REMOVE_HEAD(&pipe->up_queue, ux_next); if (pipe->up_serialise) usbd_start_next(pipe); - usbd_unlock_pipe(pipe); + if (pipe->up_dev->ud_bus->ub_usepolling == 0) + usbd_unlock_pipe(pipe); } if (!(flags & USBD_SYNCHRONOUS)) { @@ -480,7 +487,8 @@ usbd_transfer(struct usbd_xfer *xfer) } /* Sync transfer, wait for completion. */ - usbd_lock_pipe(pipe); + if (pipe->up_dev->ud_bus->ub_usepolling == 0) + usbd_lock_pipe(pipe); while (!xfer->ux_done) { if (pipe->up_dev->ud_bus->ub_usepolling) panic("usbd_transfer: not done"); @@ -503,7 +511,8 @@ usbd_transfer(struct usbd_xfer *xfer) } err = xfer->ux_status; SDT_PROBE2(usb, device, xfer, done, xfer, err); - usbd_unlock_pipe(pipe); + if (pipe->up_dev->ud_bus->ub_usepolling == 0) + usbd_unlock_pipe(pipe); return err; } @@ -1362,14 +1371,34 @@ usbd_dopoll(struct usbd_interface *iface void usbd_set_polling(struct usbd_device *dev, int on) { - if (on) - dev->ud_bus->ub_usepolling++; - else - dev->ud_bus->ub_usepolling--; - /* Kick the host controller when switching modes */ mutex_enter(dev->ud_bus->ub_lock); - dev->ud_bus->ub_methods->ubm_softint(dev->ud_bus); + if (on) { + /* + * Enabling polling. If we're enabling for the first + * time, call the softint routine on transition while + * we hold the lock and polling is still disabled, and + * then enable polling -- once polling is enabled, we + * must not hold the lock when we call the softint + * routine. + */ + KASSERT(dev->ud_bus->ub_usepolling < __type_max(char)); + if (dev->ud_bus->ub_usepolling == 0) + dev->ud_bus->ub_methods->ubm_softint(dev->ud_bus); + dev->ud_bus->ub_usepolling++; + } else { + /* + * Disabling polling. If we're disabling polling for + * the last time, disable polling first and then call + * the softint routine while we hold the lock -- until + * polling is disabled, we must not hold the lock when + * we call the softint routine. + */ + KASSERT(dev->ud_bus->ub_usepolling > 0); + dev->ud_bus->ub_usepolling--; + if (dev->ud_bus->ub_usepolling == 0) + dev->ud_bus->ub_methods->ubm_softint(dev->ud_bus); + } mutex_exit(dev->ud_bus->ub_lock); }