Re: kern/57270: panic (KASSERT) in usb

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

The following reply was made to PR kern/57270; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: bouyer%antioche.eu.org@localhost
Cc: gnats-bugs%NetBSD.org@localhost
Subject: Re: kern/57270: panic (KASSERT) in usb
Date: Wed, 15 Mar 2023 12:04:51 +0000

 > Date: Wed, 15 Mar 2023 12:39:58 +0100 (MET)
 > From: bouyer%antioche.eu.org@localhost
 >=20
 > System panicked: kernel diagnostic assertion "pipe->up_abortlwp =3D=3D NU=
 LL" failed: file "/local/armandeche2/netbsd-10/src/sys/dev/usb/usbdi.c", li=
 ne 1037 pipe->up_abortlwp=3D0xffff85745ccae640
 > crash> tr
 > __kernel_end() at 0
 > kern_reboot() at sys_reboot
 > vpanic() at vpanic+0x18d
 > kern_assert() at __x86_indirect_thunk_rax
 > usbd_ar_pipe() at usbd_ar_pipe+0x223
 > usbd_abort_pipe() at usbd_abort_pipe+0x25
 > ugen_do_close() at ugen_do_close+0x8c
 > ugenclose() at ugenclose+0x5f
 > cdev_close() at cdev_close+0x92
 
 The problem is that ugenclose and ugen_detach raced to abort the pipe.
 Aborting one pipe in two threads concurrently is forbidden.
 
 The correct approach here is:
 
 1. Split ugenclose into ugencancel and ugenclose.
 
    =3D> ugencancel will abort the pipes to wake all pending I/O and make
       it fail promptly.
 
       (ugen_detach also does cv_broadcast to wake all pending I/O, but
       I think the pipe abort can be made to suffice.)
 
    =3D> ugenclose is guaranteed that all I/O (open, read, write, ioctl)=20
       has completed, so it can safely free the resources.
 
 2. Remove the abort/drain/refcnt logic in ugen_detach.
 
    =3D> vdevgone will take care of it by revoking all the open
       instances, which will force ugencancel and ugenclose to complete
       on all of them before returning.
 
    (All the I/O refcnt logic should be able to be ripped out once ugen
    uses cancel/close.)
 
 I started drafting a change to do this last year but got sidetracked
 by per-endpoint locking and the ugen/ugenif dual use, so I focussed on
 dealing with ucom and uhid instead, which were a little simpler and
 higher-priority for my needs at the time.  Perhaps your device will
 serve as a helpful test case for this race!