New patch applied, same procedure:


se-h1# uname -v
GENERIC.MP#1

se-h1# tail -f /dev/ugen0.01

[user remove the USB device]

tail: /dev/ugen0.01: Input/output error
tail: Lost file /dev/ugen0.01: Input/output error

^C

[ no panic, back to sh ]



I did the test twice, here's the dmesg output:



se-h1# dmesg

[...]

ugen0 at uhub0 port 7 "INNO TECH USB to Serial" rev 1.10/0.02 addr 7
tsleep_nsec: tail[48559]: ugenrintr: trying to sleep zero nanoseconds
ugen0 detached
ugen0 at uhub0 port 7 "INNO TECH USB to Serial" rev 1.10/0.02 addr 7
tsleep_nsec: tail[80471]: ugenrintr: trying to sleep zero nanoseconds
ugen0 detached
ugen0 at uhub0 port 7 "INNO TECH USB to Serial" rev 1.10/0.02 addr 7



It seems to still react as we would expect.



To be more confident I did another test with less:



se-h1# less +F -f /dev/ugen0.01

--> Waiting for data...

[user remove the USB device]

--> Read error (press RETURN)

^CQ

[no panic, back to sh]




De : Visa Hankala <[email protected]>
À : [email protected]
Sujet : Re: panic at klist_remove_locked trigerred by SIGTERM on tail -f 
/dev/ugen0.01 after unplugging a USB serial interface
Date : 18/12/2022 17:14:27 Europe/Paris
Copie à : [email protected]

On Sun, Dec 18, 2022 at 02:32:09PM +0100, [email protected] wrote:
> I tested, it looks promising:
> 
> 0. Rebooted on the new GENERIC.MP built, uname -v
> 
> GENERIC.MP#0
> 
> 1. Physical plug
> 
> ugen0 at uhub0 port 7 "INNO TECH USB to Serial" rev 1.10/0.02 addr 3
> 
> 2. Read
> 
> tail -f /dev/ugen0.01
> 
> 3. Physical unplug
> 
> ugen0 detached
> 
> 4. Tail reports i/o error as expected
> 
> tail: /dev/ugen0.01: Input/output error          
> tail: Lost file /dev/ugen0.01: Input/output error
> 
> 5. ^C
> 
> No panic occurring.

Good, we are on the right track.

Below is a revised patch. Could you replace the initial patch with
this one and test again?

This new patch ensures that klist_invalidate() does get called after
vdevgone(). Making the call from ugen_do_close() might be brittle
if kqueue event registration (ugenkqfilter()) happens to race with
detaching (usbd_is_dying() possibly stops the race, though).

Index: dev/usb/ugen.c
===================================================================
RCS file: src/sys/dev/usb/ugen.c,v
retrieving revision 1.116
diff -u -p -r1.116 ugen.c
--- dev/usb/ugen.c 2 Jul 2022 08:50:42 -0000 1.116
+++ dev/usb/ugen.c 18 Dec 2022 15:48:11 -0000
@@ -798,6 +798,10 @@ ugen_detach(struct device *self, int fla
for (endptno = 0; endptno < USB_MAX_ENDPOINTS; endptno++) {
if (sc->sc_is_open[endptno])
ugen_do_close(sc, endptno, FREAD|FWRITE);
+
+ /* ugenkqfilter() always uses IN. */
+ sce = &sc->sc_endpoints[endptno][IN];
+ klist_invalidate(&sce->rsel.si_note);
}
return (0);
}

Reply via email to