Introduce `sc_mtx` mutex(9) and use it for `sc_frames_ready' and `sc_rklist' knotes list protection.
Index: sys/dev/video.c =================================================================== RCS file: /cvs/src/sys/dev/video.c,v retrieving revision 1.57 diff -u -p -r1.57 video.c --- sys/dev/video.c 2 Jul 2022 08:50:41 -0000 1.57 +++ sys/dev/video.c 27 Sep 2023 20:15:34 -0000 @@ -20,12 +20,14 @@ #include <sys/param.h> #include <sys/systm.h> #include <sys/errno.h> +#include <sys/event.h> #include <sys/ioctl.h> #include <sys/fcntl.h> #include <sys/device.h> #include <sys/vnode.h> #include <sys/kernel.h> #include <sys/malloc.h> +#include <sys/mutex.h> #include <sys/conf.h> #include <sys/proc.h> #include <sys/videoio.h> @@ -34,6 +36,11 @@ #include <uvm/uvm_extern.h> +/* + * Locks used to protect struct members and global data + * m sc_mtx + */ + #ifdef VIDEO_DEBUG int video_debug = 1; #define DPRINTF(l, x...) do { if ((l) <= video_debug) printf(x); } while (0) @@ -50,6 +57,8 @@ struct video_softc { struct process *sc_owner; /* owner process */ uint8_t sc_open; /* device opened */ + struct mutex sc_mtx; + int sc_fsize; uint8_t *sc_fbuffer; caddr_t sc_fbuffer_mmap; @@ -58,9 +67,9 @@ struct video_softc { #define VIDMODE_NONE 0 #define VIDMODE_MMAP 1 #define VIDMODE_READ 2 - int sc_frames_ready; + int sc_frames_ready; /* [m] */ - struct selinfo sc_rsel; /* read selector */ + struct klist sc_rklist; /* [m] read selector */ }; int videoprobe(struct device *, void *, void *); @@ -105,6 +114,8 @@ videoattach(struct device *parent, struc sc->sc_dev = parent; sc->sc_fbufferlen = 0; sc->sc_owner = NULL; + mtx_init(&sc->sc_mtx, IPL_MPFLOOR); + klist_init_mutex(&sc->sc_rklist, &sc->sc_mtx); if (sc->hw_if->get_bufsize) sc->sc_fbufferlen = (sc->hw_if->get_bufsize)(sc->hw_hdl); @@ -205,21 +216,28 @@ videoread(dev_t dev, struct uio *uio, in DPRINTF(1, "resid=%zu\n", uio->uio_resid); + mtx_enter(&sc->sc_mtx); + if (sc->sc_frames_ready < 1) { /* block userland read until a frame is ready */ - error = tsleep_nsec(sc, PWAIT | PCATCH, "vid_rd", INFSLP); + error = msleep_nsec(sc, &sc->sc_mtx, PWAIT | PCATCH, + "vid_rd", INFSLP); if (sc->sc_dying) error = EIO; - if (error) + if (error) { + mtx_leave(&sc->sc_mtx); return (error); + } } + sc->sc_frames_ready--; + + mtx_leave(&sc->sc_mtx); /* move no more than 1 frame to userland, as per specification */ size = ulmin(uio->uio_resid, sc->sc_fsize); if (!video_record_enable) bzero(sc->sc_fbuffer, size); error = uiomove(sc->sc_fbuffer, size, uio); - sc->sc_frames_ready--; if (error) return (error); @@ -356,7 +374,9 @@ videoioctl(dev_t dev, u_long cmd, caddr_ (struct v4l2_buffer *)data); if (!video_record_enable) bzero(sc->sc_fbuffer_mmap + vb->m.offset, vb->length); + mtx_enter(&sc->sc_mtx); sc->sc_frames_ready--; + mtx_leave(&sc->sc_mtx); break; case VIDIOC_STREAMON: if (sc->hw_if->streamon) @@ -429,11 +449,8 @@ void filt_videodetach(struct knote *kn) { struct video_softc *sc = kn->kn_hook; - int s; - s = splhigh(); - klist_remove_locked(&sc->sc_rsel.si_note, kn); - splx(s); + klist_remove(&sc->sc_rklist, kn); } int @@ -447,11 +464,39 @@ filt_videoread(struct knote *kn, long hi return (0); } +int +filt_videomodify(struct kevent *kev, struct knote *kn) +{ + struct video_softc *sc = kn->kn_hook; + int active; + + mtx_enter(&sc->sc_mtx); + active = knote_modify(kev, kn); + mtx_leave(&sc->sc_mtx); + + return (active); +} + +int +filt_videoprocess(struct knote *kn, struct kevent *kev) +{ + struct video_softc *sc = kn->kn_hook; + int active; + + mtx_enter(&sc->sc_mtx); + active = knote_process(kn, kev); + mtx_leave(&sc->sc_mtx); + + return (active); +} + const struct filterops video_filtops = { - .f_flags = FILTEROP_ISFD, + .f_flags = FILTEROP_ISFD | FILTEROP_MPSAFE, .f_attach = NULL, .f_detach = filt_videodetach, .f_event = filt_videoread, + .f_modify = filt_videomodify, + .f_process = filt_videoprocess, }; int @@ -459,7 +504,7 @@ videokqfilter(dev_t dev, struct knote *k { int unit = VIDEOUNIT(dev); struct video_softc *sc; - int s, error; + int error; KERNEL_ASSERT_LOCKED(); @@ -493,9 +538,7 @@ videokqfilter(dev_t dev, struct knote *k sc->sc_vidmode = VIDMODE_READ; } - s = splhigh(); - klist_insert_locked(&sc->sc_rsel.si_note, kn); - splx(s); + klist_insert(&sc->sc_rklist, kn); return (0); } @@ -528,13 +571,15 @@ video_intr(void *addr) struct video_softc *sc = (struct video_softc *)addr; DPRINTF(3, "video_intr sc=%p\n", sc); + mtx_enter(&sc->sc_mtx); if (sc->sc_vidmode != VIDMODE_NONE) sc->sc_frames_ready++; else printf("%s: interrupt but no streams!\n", __func__); if (sc->sc_vidmode == VIDMODE_READ) wakeup(sc); - selwakeup(&sc->sc_rsel); + knote_locked(&sc->sc_rklist, 0); + mtx_leave(&sc->sc_mtx); } int @@ -548,7 +593,9 @@ video_stop(struct video_softc *sc) error = sc->hw_if->close(sc->hw_hdl); sc->sc_vidmode = VIDMODE_NONE; + mtx_enter(&sc->sc_mtx); sc->sc_frames_ready = 0; + mtx_leave(&sc->sc_mtx); sc->sc_owner = NULL; return (error); @@ -582,7 +629,7 @@ int videodetach(struct device *self, int flags) { struct video_softc *sc = (struct video_softc *)self; - int s, maj, mn; + int maj, mn; /* locate the major number */ for (maj = 0; maj < nchrdev; maj++) @@ -593,9 +640,8 @@ videodetach(struct device *self, int fla mn = self->dv_unit; vdevgone(maj, mn, mn, VCHR); - s = splhigh(); - klist_invalidate(&sc->sc_rsel.si_note); - splx(s); + klist_invalidate(&sc->sc_rklist); + klist_free(&sc->sc_rklist); free(sc->sc_fbuffer, M_DEVBUF, sc->sc_fbufferlen);