Download raw body.
video(4): replace selinfo with klist and make `video_filtops' mp-safe
Since {,u}video(4) are no more orphaned, I like to remind about my old
diff which replaces legacy selinfo data with klist. This time selinfo is
just a wrapper around klist, but protected with kernel lock. This diff
introduces `sc_mtx' mutex(9) to protect `sc_frames_ready' and klist
data.
I usa it with:
uvideo0 at uhub0 port 7 configuration 1 interface 0 "SunplusIT Inc HD Webcam" rev 2.01/3.01 addr 3
Index: sys/dev/video.c
===================================================================
RCS file: /cvs/src/sys/dev/video.c,v
diff -u -p -r1.57 video.c
--- sys/dev/video.c 2 Jul 2022 08:50:41 -0000 1.57
+++ sys/dev/video.c 14 Dec 2024 03:51:56 -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,7 @@ 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 +66,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 +113,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 +215,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 +373,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 +448,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 +463,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 +503,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 +537,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 +570,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 +592,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 +628,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 +639,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);
video(4): replace selinfo with klist and make `video_filtops' mp-safe