Re: CVS commit: src/sys/dev/usb

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

> Module Name:    src
> Committed By:   manu
> Date:           Thu Oct  9 15:53:16 UTC 2025
> 
> Modified Files:
>         src/sys/dev/usb: usbdi.c
> 
> Log Message:
> Adjust polling transition so that it can be compatible with DDB
> 
> Upcoming changes bring console and DDB support to ucom(4).
> 
> Approved by Nick Hudson

 usbd_set_polling(struct usbd_device *dev, int on)
 {
 
-       mutex_enter(dev->ud_bus->ub_lock);
+       if (!db_active)
+               mutex_enter(dev->ud_bus->ub_lock);
...
+        * The softint routine is called after enabling polling
+        * and before disabling it, so that holding sc->sc_lock
+        * is not required. DDB needs this because it cannot wait
+        * to acquire sc->sc_lock from a suspended thread.

This change doesn't make sense; it fundamentally cannot work.

usbd_set_polling is used, via cnpollc, by ddb (and a few other things
like userconf) to switch from interrupt-based I/O to polling-based I/O
_when the system is guaranteed to be running a single thread_ either
because other threads haven't started or because ddb has already
suspended all the other CPUs.

There are two cases here:

(a) If the bus lock is not held, then there is nothing wrong with the
    code that was there before.

(b) If the bus lock is held, then it is forbidden to touch any data
    structures or hardware registers covered by the bus lock because
    they all are currently in an unpredictable, inconsistent state in
    the middle of an operation by a thread that assumes -- because it
    holds a lock on the mutex -- that it has _exclusive_ access to
    them.

Simply skipping the mutex_enter doesn't improve either situation: in
case (a), it doesn't achieve anything; in case (b), it turns a failure
or hang into a near-guarantee to corrupt some kernel data structures
and hardware registers.

So in the case where it appeared to make a difference, it made a
difference by removing the armored boot that protects you from
shooting yourself in the foot, and now you're just shooting yourself
in the foot.

I appreciate that it is frustrating to find that the lock may be held
by a thread that isn't running because ddb has suspended all the other
CPUs and the scheduler.  But just skipping the mutex_enter can't fix
that.  If you want to be able to interrupt logic in the USB stack at
any point and still use the HCI's hardware registers and the driver
data structures, you need a different approach.