On 2023 Jul 11 (Tue) at 16:40:32 +0300 (+0300), Vitaliy Makkoveev wrote:
:Use per 'wseventvar' structure `mtx' mutex(9) to protect `put' and `get'
:circular buffer indexes together with klist data. Not a big deal, but
:Xorg will not kernel lock while polling keyboard and mouse events. Also
:removed obsolete selinfo.
:
:Feedback, objections, oks?
:
Brief testing on amd64 laptop, with some games and some network and
compiles happening. Seems fine so far.
:Not related to this diff, but since 'wseventvar' members are not private
:to wscons/wsevent.c, does it make sense to add ws_ or wse_ prefix to
:them?
:
:Index: sys/dev/wscons/wsevent.c
:===================================================================
:RCS file: /cvs/src/sys/dev/wscons/wsevent.c,v
:retrieving revision 1.27
:diff -u -p -r1.27 wsevent.c
:--- sys/dev/wscons/wsevent.c 6 Jul 2023 10:16:58 -0000 1.27
:+++ sys/dev/wscons/wsevent.c 11 Jul 2023 13:36:26 -0000
:@@ -85,12 +85,16 @@
:
: void filt_wseventdetach(struct knote *);
: int filt_wseventread(struct knote *, long);
:+int filt_wseventmodify(struct kevent *, struct knote *);
:+int filt_wseventprocess(struct knote *, struct kevent *);
:
: const struct filterops wsevent_filtops = {
:- .f_flags = FILTEROP_ISFD,
:+ .f_flags = FILTEROP_ISFD | FILTEROP_MPSAFE,
: .f_attach = NULL,
: .f_detach = filt_wseventdetach,
: .f_event = filt_wseventread,
:+ .f_modify = filt_wseventmodify,
:+ .f_process = filt_wseventprocess,
: };
:
: /*
:@@ -114,6 +118,8 @@ wsevent_init(struct wseventvar *ev)
: ev->q = queue;
: ev->get = ev->put = 0;
:
:+ mtx_init_flags(&ev->mtx, IPL_MPFLOOR, "wsevmtx", 0);
:+ klist_init_mutex(&ev->klist, &ev->mtx);
: sigio_init(&ev->sigio);
:
: return (0);
:@@ -134,7 +140,7 @@ wsevent_fini(struct wseventvar *ev)
: free(ev->q, M_DEVBUF, WSEVENT_QSIZE * sizeof(struct wscons_event));
: ev->q = NULL;
:
:- klist_invalidate(&ev->sel.si_note);
:+ klist_invalidate(&ev->klist);
:
: sigio_free(&ev->sigio);
: }
:@@ -146,8 +152,8 @@ wsevent_fini(struct wseventvar *ev)
: int
: wsevent_read(struct wseventvar *ev, struct uio *uio, int flags)
: {
:- int s, error;
:- u_int cnt;
:+ int error, notwrap = 0;
:+ u_int cnt, tcnt, get;
: size_t n;
:
: /*
:@@ -155,17 +161,19 @@ wsevent_read(struct wseventvar *ev, stru
: */
: if (uio->uio_resid < sizeof(struct wscons_event))
: return (EMSGSIZE); /* ??? */
:- s = splwsevent();
:+
:+ mtx_enter(&ev->mtx);
:+
: while (ev->get == ev->put) {
: if (flags & IO_NDELAY) {
:- splx(s);
:+ mtx_leave(&ev->mtx);
: return (EWOULDBLOCK);
: }
: ev->wanted = 1;
:- error = tsleep_nsec(ev, PWSEVENT | PCATCH,
:+ error = msleep_nsec(ev, &ev->mtx, PWSEVENT | PCATCH,
: "wsevent_read", INFSLP);
: if (error) {
:- splx(s);
:+ mtx_leave(&ev->mtx);
: return (error);
: }
: }
:@@ -177,37 +185,43 @@ wsevent_read(struct wseventvar *ev, stru
: cnt = WSEVENT_QSIZE - ev->get; /* events in [get..QSIZE) */
: else
: cnt = ev->put - ev->get; /* events in [get..put) */
:- splx(s);
:+
: n = howmany(uio->uio_resid, sizeof(struct wscons_event));
: if (cnt > n)
: cnt = n;
:- error = uiomove((caddr_t)&ev->q[ev->get],
:- cnt * sizeof(struct wscons_event), uio);
:+
:+ get = ev->get;
:+ tcnt = ev->put;
: n -= cnt;
:+
:+ if ((ev->get = (ev->get + cnt) % WSEVENT_QSIZE) != 0 || n == 0 ||
:+ tcnt == 0) {
:+ notwrap = 1;
:+ } else {
:+ if (tcnt > n)
:+ tcnt = n;
:+ ev->get = tcnt;
:+ }
:+
:+ mtx_leave(&ev->mtx);
:+
:+ error = uiomove((caddr_t)&ev->q[get],
:+ cnt * sizeof(struct wscons_event), uio);
: /*
: * If we do not wrap to 0, used up all our space, or had an error,
: * stop. Otherwise move from front of queue to put index, if there
: * is anything there to move.
: */
:- if ((ev->get = (ev->get + cnt) % WSEVENT_QSIZE) != 0 ||
:- n == 0 || error || (cnt = ev->put) == 0)
:+ if (notwrap || error)
: return (error);
:- if (cnt > n)
:- cnt = n;
: error = uiomove((caddr_t)&ev->q[0],
:- cnt * sizeof(struct wscons_event), uio);
:- ev->get = cnt;
:+ tcnt * sizeof(struct wscons_event), uio);
: return (error);
: }
:
: int
: wsevent_kqfilter(struct wseventvar *ev, struct knote *kn)
: {
:- struct klist *klist;
:- int s;
:-
:- klist = &ev->sel.si_note;
:-
: switch (kn->kn_filter) {
: case EVFILT_READ:
: kn->kn_fop = &wsevent_filtops;
:@@ -217,10 +231,7 @@ wsevent_kqfilter(struct wseventvar *ev,
: }
:
: kn->kn_hook = ev;
:-
:- s = splwsevent();
:- klist_insert_locked(klist, kn);
:- splx(s);
:+ klist_insert(&ev->klist, kn);
:
: return (0);
: }
:@@ -229,12 +240,8 @@ void
: filt_wseventdetach(struct knote *kn)
: {
: struct wseventvar *ev = kn->kn_hook;
:- struct klist *klist = &ev->sel.si_note;
:- int s;
:
:- s = splwsevent();
:- klist_remove_locked(klist, kn);
:- splx(s);
:+ klist_remove(&ev->klist, kn);
: }
:
: int
:@@ -242,6 +249,8 @@ filt_wseventread(struct knote *kn, long
: {
: struct wseventvar *ev = kn->kn_hook;
:
:+ MUTEX_ASSERT_LOCKED(&ev->mtx);
:+
: if (ev->get == ev->put)
: return (0);
:
:@@ -251,4 +260,30 @@ filt_wseventread(struct knote *kn, long
: kn->kn_data = (WSEVENT_QSIZE - ev->get) + ev->put;
:
: return (1);
:+}
:+
:+int
:+filt_wseventmodify(struct kevent *kev, struct knote *kn)
:+{
:+ struct wseventvar *ev = kn->kn_hook;
:+ int active;
:+
:+ mtx_enter(&ev->mtx);
:+ active = knote_modify(kev, kn);
:+ mtx_leave(&ev->mtx);
:+
:+ return (active);
:+}
:+
:+int
:+filt_wseventprocess(struct knote *kn, struct kevent *kev)
:+{
:+ struct wseventvar *ev = kn->kn_hook;
:+ int active;
:+
:+ mtx_enter(&ev->mtx);
:+ active = knote_process(kn, kev);
:+ mtx_leave(&ev->mtx);
:+
:+ return (active);
: }
:Index: sys/dev/wscons/wseventvar.h
:===================================================================
:RCS file: /cvs/src/sys/dev/wscons/wseventvar.h,v
:retrieving revision 1.11
:diff -u -p -r1.11 wseventvar.h
:--- sys/dev/wscons/wseventvar.h 2 Jul 2022 08:50:42 -0000 1.11
:+++ sys/dev/wscons/wseventvar.h 11 Jul 2023 13:36:26 -0000
:@@ -71,7 +71,9 @@
: * @(#)event_var.h 8.1 (Berkeley) 6/11/93
: */
:
:+#include <sys/mutex.h>
: #include <sys/sigio.h>
:+#include <sys/signalvar.h>
:
: /*
: * Internal "wscons_event" queue interface for the keyboard and mouse drivers.
:@@ -83,25 +85,29 @@
: #define WSEVENT_QSIZE 256 /* may need tuning; this uses 2k */
:
: struct wseventvar {
:+ struct mutex mtx;
: u_int get; /* get (read) index (modified synchronously) */
: volatile u_int put; /* put (write) index (modified by interrupt) */
:- struct selinfo sel; /* process selecting */
:+ struct klist klist; /* process selecting */
: struct sigio_ref sigio; /* async I/O registration */
: int wanted; /* wake up on input ready */
: int async; /* send SIGIO on input ready */
: struct wscons_event *q; /* circular buffer (queue) of events */
: };
:
:-#define splwsevent() spltty()
:+static inline void
:+wsevent_wakeup(struct wseventvar *ev)
:+{
:+ MUTEX_ASSERT_LOCKED(&ev->mtx);
:
:-#define WSEVENT_WAKEUP(ev) { \
:- selwakeup(&(ev)->sel); \
:- if ((ev)->wanted) { \
:- (ev)->wanted = 0; \
:- wakeup((caddr_t)(ev)); \
:- } \
:- if ((ev)->async) \
:- pgsigio(&(ev)->sigio, SIGIO, 0); \
:+ knote_locked(&ev->klist, 0);
:+
:+ if (ev->wanted) {
:+ ev->wanted = 0;
:+ wakeup((caddr_t)ev);
:+ }
:+ if (ev->async)
:+ pgsigio(&ev->sigio, SIGIO, 0);
: }
:
: int wsevent_init(struct wseventvar *);
:Index: sys/dev/wscons/wskbd.c
:===================================================================
:RCS file: /cvs/src/sys/dev/wscons/wskbd.c,v
:retrieving revision 1.115
:diff -u -p -r1.115 wskbd.c
:--- sys/dev/wscons/wskbd.c 9 Jul 2023 08:02:14 -0000 1.115
:+++ sys/dev/wscons/wskbd.c 11 Jul 2023 13:36:26 -0000
:@@ -637,10 +637,12 @@ wskbd_detach(struct device *self, int f
: if (evar != NULL) {
: s = spltty();
: if (--sc->sc_refcnt >= 0) {
:+ mtx_enter(&evar->mtx);
: /* Wake everyone by generating a dummy event. */
: if (++evar->put >= WSEVENT_QSIZE)
: evar->put = 0;
:- WSEVENT_WAKEUP(evar);
:+ wsevent_wakeup(evar);
:+ mtx_leave(&evar->mtx);
: /* Wait for processes to go away. */
: if (tsleep_nsec(sc, PZERO, "wskdet", SEC_TO_NSEC(60)))
: printf("wskbd_detach: %s didn't detach\n",
:@@ -751,10 +753,12 @@ wskbd_deliver_event(struct wskbd_softc *
: }
: #endif
:
:+ mtx_enter(&evar->mtx);
: put = evar->put;
: ev = &evar->q[put];
: put = (put + 1) % WSEVENT_QSIZE;
: if (put == evar->get) {
:+ mtx_leave(&evar->mtx);
: log(LOG_WARNING, "%s: event queue overflow\n",
: sc->sc_base.me_dv.dv_xname);
: return;
:@@ -763,7 +767,8 @@ wskbd_deliver_event(struct wskbd_softc *
: ev->value = value;
: nanotime(&ev->time);
: evar->put = put;
:- WSEVENT_WAKEUP(evar);
:+ wsevent_wakeup(evar);
:+ mtx_leave(&evar->mtx);
: }
:
: #ifdef WSDISPLAY_COMPAT_RAWKBD
:Index: sys/dev/wscons/wsmouse.c
:===================================================================
:RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v
:retrieving revision 1.70
:diff -u -p -r1.70 wsmouse.c
:--- sys/dev/wscons/wsmouse.c 16 Oct 2022 18:23:44 -0000 1.70
:+++ sys/dev/wscons/wsmouse.c 11 Jul 2023 13:36:26 -0000
:@@ -264,10 +264,12 @@ wsmouse_detach(struct device *self, int
: if (evar != NULL) {
: s = spltty();
: if (--sc->sc_refcnt >= 0) {
:+ mtx_enter(&evar->mtx);
: /* Wake everyone by generating a dummy event. */
: if (++evar->put >= WSEVENT_QSIZE)
: evar->put = 0;
:- WSEVENT_WAKEUP(evar);
:+ wsevent_wakeup(evar);
:+ mtx_leave(&evar->mtx);
: /* Wait for processes to go away. */
: if (tsleep_nsec(sc, PZERO, "wsmdet", SEC_TO_NSEC(60)))
: printf("wsmouse_detach: %s didn't detach\n",
:@@ -953,7 +955,7 @@ wsmouse_mt_convert(struct device *sc)
: }
:
: void
:-wsmouse_evq_put(struct evq_access *evq, int ev_type, int ev_value)
:+wsmouse_evq_put_locked(struct evq_access *evq, int ev_type, int ev_value)
: {
: struct wscons_event *ev;
: int space;
:@@ -971,6 +973,13 @@ wsmouse_evq_put(struct evq_access *evq,
: }
: }
:
:+void
:+wsmouse_evq_put(struct evq_access *evq, int ev_type, int ev_value)
:+{
:+ mtx_enter(&evq->evar->mtx);
:+ wsmouse_evq_put_locked(evq, ev_type, ev_value);
:+ mtx_leave(&evq->evar->mtx);
:+}
:
: void
: wsmouse_btn_sync(struct btn_state *btn, struct evq_access *evq)
:@@ -1141,7 +1150,9 @@ wsmouse_input_sync(struct device *sc)
: evq.evar = *input->evar;
: if (evq.evar == NULL)
: return;
:+ mtx_enter(&evq.evar->mtx);
: evq.put = evq.evar->put;
:+ mtx_leave(&evq.evar->mtx);
: evq.result = EVQ_RESULT_NONE;
: getnanotime(&evq.ts);
:
:@@ -1179,14 +1190,16 @@ wsmouse_input_sync(struct device *sc)
: /* No MT events are generated yet. */
:
: if (evq.result == EVQ_RESULT_SUCCESS) {
:- wsmouse_evq_put(&evq, WSCONS_EVENT_SYNC, 0);
:+ mtx_enter(&evq.evar->mtx);
:+ wsmouse_evq_put_locked(&evq, WSCONS_EVENT_SYNC, 0);
: if (evq.result == EVQ_RESULT_SUCCESS) {
: if (input->flags & LOG_EVENTS) {
: wsmouse_log_events(input, &evq);
: }
: evq.evar->put = evq.put;
:- WSEVENT_WAKEUP(evq.evar);
:+ wsevent_wakeup(evq.evar);
: }
:+ mtx_leave(&evq.evar->mtx);
: }
:
: if (evq.result != EVQ_RESULT_OVERFLOW)
:Index: sys/dev/wscons/wsmouseinput.h
:===================================================================
:RCS file: /cvs/src/sys/dev/wscons/wsmouseinput.h,v
:retrieving revision 1.15
:diff -u -p -r1.15 wsmouseinput.h
:--- sys/dev/wscons/wsmouseinput.h 21 Mar 2021 16:20:49 -0000 1.15
:+++ sys/dev/wscons/wsmouseinput.h 11 Jul 2023 13:36:26 -0000
:@@ -184,6 +184,7 @@ struct evq_access {
:
:
: void wsmouse_evq_put(struct evq_access *, int, int);
:+void wsmouse_evq_put_locked(struct evq_access *, int, int);
: void wsmouse_log_events(struct wsmouseinput *, struct evq_access *);
: int wsmouse_hysteresis(struct wsmouseinput *, struct position *);
: void wsmouse_input_reset(struct wsmouseinput *);
:Index: sys/dev/wscons/wsmux.c
:===================================================================
:RCS file: /cvs/src/sys/dev/wscons/wsmux.c,v
:retrieving revision 1.56
:diff -u -p -r1.56 wsmux.c
:--- sys/dev/wscons/wsmux.c 2 Jul 2022 08:50:42 -0000 1.56
:+++ sys/dev/wscons/wsmux.c 11 Jul 2023 13:36:26 -0000
:@@ -385,7 +385,7 @@ wsmux_do_ioctl(struct device *dv, u_long
: struct wsmux_softc *sc = (struct wsmux_softc *)dv;
: struct wsevsrc *me;
: int error, ok;
:- int s, put, get, n;
:+ int put, get, n;
: struct wseventvar *evar;
: struct wscons_event *ev;
: struct wsmux_device_list *l;
:@@ -415,13 +415,12 @@ wsmux_do_ioctl(struct device *dv, u_long
: return (0);
: }
:
:- s = spltty();
:+ mtx_enter(&evar->mtx);
: get = evar->get;
: put = evar->put;
: ev = &evar->q[put];
: if (++put % WSEVENT_QSIZE == get) {
:- put--;
:- splx(s);
:+ mtx_leave(&evar->mtx);
: return (ENOSPC);
: }
: if (put >= WSEVENT_QSIZE)
:@@ -429,8 +428,9 @@ wsmux_do_ioctl(struct device *dv, u_long
: *ev = *(struct wscons_event *)data;
: nanotime(&ev->time);
: evar->put = put;
:- WSEVENT_WAKEUP(evar);
:- splx(s);
:+ wsevent_wakeup(evar);
:+ mtx_leave(&evar->mtx);
:+
: return (0);
: case WSMUXIO_ADD_DEVICE:
: #define d ((struct wsmux_device *)data)
:Index: sys/dev/wscons/wstpad.c
:===================================================================
:RCS file: /cvs/src/sys/dev/wscons/wstpad.c,v
:retrieving revision 1.32
:diff -u -p -r1.32 wstpad.c
:--- sys/dev/wscons/wstpad.c 2 Jul 2023 21:44:04 -0000 1.32
:+++ sys/dev/wscons/wstpad.c 11 Jul 2023 13:36:26 -0000
:@@ -931,20 +931,22 @@ wstpad_tap_timeout(void *p)
: tp->tap.state = TAP_DETECT;
: }
: if (ev) {
:+ mtx_enter(&evq.evar->mtx);
: evq.put = evq.evar->put;
: evq.result = EVQ_RESULT_NONE;
: getnanotime(&evq.ts);
:- wsmouse_evq_put(&evq, ev, btn);
:- wsmouse_evq_put(&evq, SYNC_EV, 0);
:+ wsmouse_evq_put_locked(&evq, ev, btn);
:+ wsmouse_evq_put_locked(&evq, SYNC_EV, 0);
: if (evq.result == EVQ_RESULT_SUCCESS) {
: if (input->flags & LOG_EVENTS) {
: wsmouse_log_events(input, &evq);
: }
: evq.evar->put = evq.put;
:- WSEVENT_WAKEUP(evq.evar);
:+ wsevent_wakeup(evq.evar);
: } else {
: input->sbtn.sync |= tp->tap.button;
: }
:+ mtx_leave(&evq.evar->mtx);
: }
: }
: splx(s);
:
--
Mosher's Law of Software Engineering:
Don't worry if it doesn't work right.
If everything did, you'd be out of a job.
