On 02/02/20(Sun) 09:58, Visa Hankala wrote:
> tty(4) uses custom code for revoking knotes. It should be changed to use
> klist_invalidate() to handle the revocation in one place. The following
> diff does that.
Great, your diff is ok mpi@
> In addition, the patch makes the code store the tty context pointer
> in kn_hook directly. This simplifies the code.
Does that mean we'll need some reference counting when getting kqueue
out of the KERNEL_LOCK()? Who is the owner of `kn_hook'?
> Unlike ttkqflush(), klist_invalidate() can sleep. However, that should
> not be a problem because the callers of ttyfree() call vdevgone() that
> can sleep as well.
It took me some time to audit com_detach() because ttyfree() isn't the
last call. Most of the *_detach() functions have similar problems: they
assume the execution of the code is atomic and free data structures
before unregistering handlers.
Diff below improves that, ok?
Index: dev/ic/com.c
===================================================================
RCS file: /cvs/src/sys/dev/ic/com.c,v
retrieving revision 1.170
diff -u -p -r1.170 com.c
--- dev/ic/com.c 19 Jul 2019 00:17:15 -0000 1.170
+++ dev/ic/com.c 4 Feb 2020 11:27:06 -0000
@@ -186,14 +186,14 @@ com_detach(struct device *self, int flag
mn |= 0x80;
vdevgone(maj, mn, mn, VCHR);
+ timeout_del(&sc->sc_dtr_tmo);
+ timeout_del(&sc->sc_diag_tmo);
+ softintr_disestablish(sc->sc_si);
+
/* Detach and free the tty. */
if (sc->sc_tty) {
ttyfree(sc->sc_tty);
}
-
- timeout_del(&sc->sc_dtr_tmo);
- timeout_del(&sc->sc_diag_tmo);
- softintr_disestablish(sc->sc_si);
return (0);
}
