Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev various clean ups for midi and sequencer:
details: https://anonhg.NetBSD.org/src/rev/98cd97069ff2
branches: trunk
changeset: 335029:98cd97069ff2
user: mrg <mrg%NetBSD.org@localhost>
date: Mon Dec 22 07:02:22 2014 +0000
description:
various clean ups for midi and sequencer:
midi specific:
- add reference counting for midi operations, and ensure that
detach waits for other threads to complete before tearing
down the device completely.
- in detach, halt midi callouts before destroying them
- re-check sc->dying after sleeping in midiread()
- in real_writebytes(), make sure we're open and not dying
- make sure we drop the interrupt lock before calling any code
that may want to check thread locks. this is now safe due to
the above changes.
sequencer specific:
- avoid caching the midi softc in the sequencer softc. instead,
every time we want to use it, look it up again and make sure
it still exists.
this fixes various crashes i've seen in the usb midi code when
detaching the umidi while it is active.
diffstat:
sys/dev/midi.c | 111 +++++++++++++++++++++++++++++++++++++++++-------
sys/dev/midivar.h | 4 +-
sys/dev/sequencer.c | 48 +++++++++++++-------
sys/dev/sequencervar.h | 3 +-
4 files changed, 129 insertions(+), 37 deletions(-)
diffs (truncated from 485 to 300 lines):
diff -r 24339e68b4b2 -r 98cd97069ff2 sys/dev/midi.c
--- a/sys/dev/midi.c Mon Dec 22 04:23:58 2014 +0000
+++ b/sys/dev/midi.c Mon Dec 22 07:02:22 2014 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: midi.c,v 1.81 2014/07/25 08:10:35 dholland Exp $ */
+/* $NetBSD: midi.c,v 1.82 2014/12/22 07:02:22 mrg Exp $ */
/*
* Copyright (c) 1998, 2008 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: midi.c,v 1.81 2014/07/25 08:10:35 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: midi.c,v 1.82 2014/12/22 07:02:22 mrg Exp $");
#include "midi.h"
#include "sequencer.h"
@@ -204,6 +204,11 @@
mutex_enter(sc->lock);
sc->dying = 1;
+
+ if (--sc->refcnt >= 0) {
+ /* Wake anything? */
+ (void)cv_timedwait(&sc->detach_cv, sc->lock, hz * 60);
+ }
cv_broadcast(&sc->wchan);
cv_broadcast(&sc->rchan);
mutex_exit(sc->lock);
@@ -236,8 +241,17 @@
sc->sih = NULL;
}
+ mutex_enter(sc->lock);
+ callout_halt(&sc->xmt_asense_co, sc->lock);
+ callout_halt(&sc->rcv_asense_co, sc->lock);
+ mutex_exit(sc->lock);
+
+ callout_destroy(&sc->xmt_asense_co);
+ callout_destroy(&sc->rcv_asense_co);
+
cv_destroy(&sc->wchan);
cv_destroy(&sc->rchan);
+ cv_destroy(&sc->detach_cv);
return (0);
}
@@ -266,9 +280,11 @@
cv_init(&sc->rchan, "midird");
cv_init(&sc->wchan, "midiwr");
+ cv_init(&sc->detach_cv, "mididet");
sc->dying = 0;
sc->isopen = 0;
+ sc->refcnt = 0;
mutex_enter(&hwif_softc_lock);
mutex_enter(sc->lock);
@@ -952,6 +968,10 @@
mutex_enter(sc->lock);
if (error)
break;
+ if (sc->dying) {
+ error = EIO;
+ break;
+ }
appetite -= buf_end - buf_cur;
buf_cur = mb->buf;
}
@@ -961,6 +981,10 @@
mutex_enter(sc->lock);
if (error)
break;
+ if (sc->dying) {
+ error = EIO;
+ break;
+ }
buf_cur += appetite;
}
@@ -1306,9 +1330,15 @@
enum fst_form form;
MIDI_BUF_DECLARE(idx);
MIDI_BUF_DECLARE(buf);
+ int error;
KASSERT(mutex_owned(sc->lock));
+ if (sc->dying || !sc->isopen)
+ return EIO;
+
+ sc->refcnt++;
+
iend = ibuf + cc;
mb = &sc->outbuf;
arming = 0;
@@ -1329,9 +1359,6 @@
MIDI_BUF_PRODUCER_INIT(mb,idx);
MIDI_BUF_PRODUCER_INIT(mb,buf);
-
- if (sc->dying)
- return EIO;
while (ibuf < iend) {
got = midi_fst(&sc->xmt, *ibuf, form);
@@ -1341,7 +1368,8 @@
continue;
case FST_ERR:
case FST_HUH:
- return EPROTO;
+ error = EPROTO;
+ goto out;
case FST_CHN:
case FST_CHV: /* only occurs in VCOMP form */
case FST_COM:
@@ -1366,8 +1394,10 @@
if (idx_cur == idx_lim || count > buf_lim - buf_cur) {
MIDI_BUF_PRODUCER_REFRESH(mb,idx); /* get the most */
MIDI_BUF_PRODUCER_REFRESH(mb,buf); /* current facts */
- if (idx_cur == idx_lim || count > buf_lim - buf_cur)
- return EWOULDBLOCK; /* caller's problem */
+ if (idx_cur == idx_lim || count > buf_lim - buf_cur) {
+ error = EWOULDBLOCK; /* caller's problem */
+ goto out;
+ }
}
*idx_cur++ = PACK_MB_IDX(got,count);
MIDI_BUF_WRAP(idx);
@@ -1394,7 +1424,14 @@
callout_stop(&sc->xmt_asense_co);
arming = 1;
}
- return arming ? midi_start_output(sc) : 0;
+
+ error = arming ? midi_start_output(sc) : 0;
+
+out:
+ if (--sc->refcnt < 0)
+ cv_broadcast(&sc->detach_cv);
+
+ return error;
}
static int
@@ -1422,6 +1459,9 @@
mutex_exit(sc->lock);
return EIO;
}
+
+ sc->refcnt++;
+
mb = &sc->outbuf;
error = 0;
while (uio->uio_resid > 0 && !error) {
@@ -1445,7 +1485,8 @@
* convert this to success with a short count.
*/
mutex_exit(sc->lock);
- return EWOULDBLOCK;
+ error = EWOULDBLOCK;
+ goto out;
}
if (pollout) {
mutex_exit(sc->lock);
@@ -1460,8 +1501,7 @@
* EINTR and ERESTART properly here, changing to
* a short count if something transferred.
*/
- mutex_exit(sc->lock);
- return error;
+ goto out;
}
}
@@ -1514,6 +1554,11 @@
DPRINTFN(8,("midiwrite: uio_resid now %zu, props=%d\n",
uio->uio_resid, sc->props));
}
+
+out:
+ if (--sc->refcnt < 0)
+ cv_broadcast(&sc->detach_cv);
+
mutex_exit(sc->lock);
return error;
}
@@ -1529,6 +1574,9 @@
device_lookup_private(&midi_cd, unit);
int error;
+ if (!sc)
+ return EIO;
+
DPRINTFN(7, ("midi_writebytes: %p, unit=%d, cc=%d %#02x %#02x %#02x\n",
sc, unit, cc, bf[0], bf[1], bf[2]));
@@ -1554,6 +1602,9 @@
hw = sc->hw_if;
error = 0;
+ mutex_enter(sc->lock);
+ sc->refcnt++;
+
DPRINTFN(5,("midiioctl: %p cmd=0x%08lx\n", sc, cmd));
switch (cmd) {
@@ -1574,13 +1625,12 @@
* read of m < n, fewer than m bytes may be read to ensure the
* read ends at a message boundary.
*/
- mutex_enter(sc->lock);
MIDI_BUF_CONSUMER_INIT(&sc->inbuf,buf);
*(int *)addr = buf_lim - buf_cur;
- mutex_exit(sc->lock);
break;
case FIOASYNC:
+ mutex_exit(sc->lock);
mutex_enter(proc_lock);
if (*(int *)addr) {
if (sc->async) {
@@ -1594,6 +1644,7 @@
sc->async = 0;
}
mutex_exit(proc_lock);
+ mutex_enter(sc->lock);
break;
#if 0
@@ -1607,20 +1658,24 @@
#ifdef MIDI_SAVE
case MIDI_GETSAVE:
+ mutex_exit(sc->lock);
error = copyout(&midisave, *(void **)addr, sizeof midisave);
+ mutex_enter(sc->lock);
break;
#endif
default:
if (hw->ioctl != NULL) {
- mutex_enter(sc->lock);
error = hw->ioctl(sc->hw_hdl, cmd, addr, flag, l);
- mutex_exit(sc->lock);
} else {
error = EINVAL;
}
break;
}
+
+ if (--sc->refcnt < 0)
+ cv_broadcast(&sc->detach_cv);
+ mutex_exit(sc->lock);
return error;
}
@@ -1643,6 +1698,9 @@
mutex_exit(sc->lock);
return POLLHUP;
}
+
+ sc->refcnt++;
+
if ((events & (POLLIN | POLLRDNORM)) != 0) {
MIDI_BUF_CONSUMER_INIT(&sc->inbuf, idx);
if (idx_cur < idx_lim)
@@ -1658,6 +1716,10 @@
else
selrecord(l, &sc->wsel);
}
+
+ if (--sc->refcnt < 0)
+ cv_broadcast(&sc->detach_cv);
+
mutex_exit(sc->lock);
return revents;
@@ -1709,6 +1771,10 @@
MIDI_BUF_DECLARE(idx);
MIDI_BUF_DECLARE(buf);
+ mutex_exit(sc->lock);
+ sc->refcnt++;
+ mutex_enter(sc->lock);
+
(void)idx_end; (void)buf_end;
if (hint != NOTE_SUBMIT)
mutex_enter(sc->lock);
@@ -1719,6 +1785,13 @@
kn->kn_data = idx_lim - idx_cur;
if (hint != NOTE_SUBMIT)
mutex_exit(sc->lock);
+
+ // XXXMRG -- move this up, avoid the relock?
+ mutex_enter(sc->lock);
+ if (--sc->refcnt < 0)
+ cv_broadcast(&sc->detach_cv);
Home |
Main Index |
Thread Index |
Old Index