Module Name: src
Committed By: mrg
Date: Mon Dec 22 07:02:22 UTC 2014
Modified Files:
src/sys/dev: midi.c midivar.h sequencer.c sequencervar.h
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.81 -r1.82 src/sys/dev/midi.c
cvs rdiff -u -r1.19 -r1.20 src/sys/dev/midivar.h
cvs rdiff -u -r1.59 -r1.60 src/sys/dev/sequencer.c
cvs rdiff -u -r1.16 -r1.17 src/sys/dev/sequencervar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/midi.c
diff -u src/sys/dev/midi.c:1.81 src/sys/dev/midi.c:1.82
--- src/sys/dev/midi.c:1.81 Fri Jul 25 08:10:35 2014
+++ src/sys/dev/midi.c Mon Dec 22 07:02:22 2014
@@ -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 @@ mididetach(device_t self, int flags)
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 @@ mididetach(device_t self, int flags)
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 @@ midi_attach(struct midi_softc *sc)
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 @@ midiread(dev_t dev, struct uio *uio, int
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 @@ midiread(dev_t dev, struct uio *uio, int
mutex_enter(sc->lock);
if (error)
break;
+ if (sc->dying) {
+ error = EIO;
+ break;
+ }
buf_cur += appetite;
}
@@ -1306,9 +1330,15 @@ real_writebytes(struct midi_softc *sc, u
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 @@ real_writebytes(struct midi_softc *sc, u
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 @@ real_writebytes(struct midi_softc *sc, u
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 @@ real_writebytes(struct midi_softc *sc, u
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 @@ real_writebytes(struct midi_softc *sc, u
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 @@ midiwrite(dev_t dev, struct uio *uio, in
mutex_exit(sc->lock);
return EIO;
}
+
+ sc->refcnt++;
+
mb = &sc->outbuf;
error = 0;
while (uio->uio_resid > 0 && !error) {
@@ -1445,7 +1485,8 @@ midiwrite(dev_t dev, struct uio *uio, in
* 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 @@ midiwrite(dev_t dev, struct uio *uio, in
* 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 @@ midiwrite(dev_t dev, struct uio *uio, in
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 @@ midi_writebytes(int unit, u_char *bf, in
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 @@ midiioctl(dev_t dev, u_long cmd, void *a
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 @@ midiioctl(dev_t dev, u_long cmd, void *a
* 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 @@ midiioctl(dev_t dev, u_long cmd, void *a
sc->async = 0;
}
mutex_exit(proc_lock);
+ mutex_enter(sc->lock);
break;
#if 0
@@ -1607,20 +1658,24 @@ midiioctl(dev_t dev, u_long cmd, void *a
#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 @@ midipoll(dev_t dev, int events, struct l
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 @@ midipoll(dev_t dev, int events, struct l
else
selrecord(l, &sc->wsel);
}
+
+ if (--sc->refcnt < 0)
+ cv_broadcast(&sc->detach_cv);
+
mutex_exit(sc->lock);
return revents;
@@ -1709,6 +1771,10 @@ filt_midiwrite(struct knote *kn, long hi
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 @@ filt_midiwrite(struct knote *kn, long hi
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);
+ mutex_exit(sc->lock);
+
return (kn->kn_data > 0);
}
@@ -1732,6 +1805,10 @@ midikqfilter(dev_t dev, struct knote *kn
device_lookup_private(&midi_cd, MIDIUNIT(dev));
struct klist *klist;
+ mutex_exit(sc->lock);
+ sc->refcnt++;
+ mutex_enter(sc->lock);
+
switch (kn->kn_filter) {
case EVFILT_READ:
klist = &sc->rsel.sel_klist;
@@ -1751,6 +1828,8 @@ midikqfilter(dev_t dev, struct knote *kn
mutex_enter(sc->lock);
SLIST_INSERT_HEAD(klist, kn, kn_selnext);
+ if (--sc->refcnt < 0)
+ cv_broadcast(&sc->detach_cv);
mutex_exit(sc->lock);
return (0);
Index: src/sys/dev/midivar.h
diff -u src/sys/dev/midivar.h:1.19 src/sys/dev/midivar.h:1.20
--- src/sys/dev/midivar.h:1.19 Thu Apr 5 20:25:53 2012
+++ src/sys/dev/midivar.h Mon Dec 22 07:02:22 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: midivar.h,v 1.19 2012/04/05 20:25:53 plunky Exp $ */
+/* $NetBSD: midivar.h,v 1.20 2014/12/22 07:02:22 mrg Exp $ */
/*
* Copyright (c) 1998, 2008 The NetBSD Foundation, Inc.
@@ -189,6 +189,8 @@ struct midi_softc {
struct midi_buffer outbuf;
struct midi_buffer inbuf;
int props;
+ int refcnt;
+ kcondvar_t detach_cv;
kcondvar_t rchan;
kcondvar_t wchan;
kmutex_t *lock;
Index: src/sys/dev/sequencer.c
diff -u src/sys/dev/sequencer.c:1.59 src/sys/dev/sequencer.c:1.60
--- src/sys/dev/sequencer.c:1.59 Fri Jul 25 08:10:35 2014
+++ src/sys/dev/sequencer.c Mon Dec 22 07:02:22 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: sequencer.c,v 1.59 2014/07/25 08:10:35 dholland Exp $ */
+/* $NetBSD: sequencer.c,v 1.60 2014/12/22 07:02:22 mrg Exp $ */
/*
* Copyright (c) 1998, 2008 The NetBSD Foundation, Inc.
@@ -55,7 +55,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sequencer.c,v 1.59 2014/07/25 08:10:35 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sequencer.c,v 1.60 2014/12/22 07:02:22 mrg Exp $");
#include "sequencer.h"
@@ -174,7 +174,9 @@ static LIST_HEAD(, sequencer_softc) sequ
static kmutex_t sequencer_lock;
static void
-sequencerdestroy(struct sequencer_softc *sc) {
+sequencerdestroy(struct sequencer_softc *sc)
+{
+ callout_halt(&sc->sc_callout, &sc->lock);
callout_destroy(&sc->sc_callout);
softint_disestablish(sc->sih);
cv_destroy(&sc->rchan);
@@ -186,7 +188,8 @@ sequencerdestroy(struct sequencer_softc
}
static struct sequencer_softc *
-sequencercreate(int unit) {
+sequencercreate(int unit)
+{
struct sequencer_softc *sc = kmem_zalloc(sizeof(*sc), KM_SLEEP);
if (sc == NULL) {
#ifdef DIAGNOSTIC
@@ -212,7 +215,8 @@ sequencercreate(int unit) {
static struct sequencer_softc *
-sequencerget(int unit) {
+sequencerget(int unit)
+{
struct sequencer_softc *sc;
if (unit < 0) {
#ifdef DIAGNOSTIC
@@ -238,7 +242,8 @@ sequencerget(int unit) {
#ifdef notyet
static void
-sequencerput(struct sequencer_softc *sc) {
+sequencerput(struct sequencer_softc *sc)
+{
mutex_enter(&sequencer_lock);
LIST_REMOVE(sc, sc_link);
mutex_exit(&sequencer_lock);
@@ -346,10 +351,14 @@ sequenceropen(dev_t dev, int flags, int
/* Only now redirect input from MIDI devices. */
for (unit = 0; unit < sc->nmidi; unit++) {
- msc = sc->devs[unit]->msc;
- mutex_enter(msc->lock);
- msc->seqopen = 1;
- mutex_exit(msc->lock);
+ extern struct cfdriver midi_cd;
+
+ msc = device_lookup_private(&midi_cd, unit);
+ if (msc) {
+ mutex_enter(msc->lock);
+ msc->seqopen = 1;
+ mutex_exit(msc->lock);
+ }
}
seq_reset(sc);
@@ -440,10 +449,14 @@ sequencerclose(dev_t dev, int flags, int
}
/* Bin input from MIDI devices. */
for (unit = 0; unit < sc->nmidi; unit++) {
- msc = sc->devs[unit]->msc;
- mutex_enter(msc->lock);
- msc->seqopen = 0;
- mutex_exit(msc->lock);
+ extern struct cfdriver midi_cd;
+
+ msc = device_lookup_private(&midi_cd, unit);
+ if (msc) {
+ mutex_enter(msc->lock);
+ msc->seqopen = 0;
+ mutex_exit(msc->lock);
+ }
}
mutex_exit(&sc->lock);
@@ -971,7 +984,7 @@ seq_reset(struct sequencer_softc *sc)
KASSERT(mutex_owned(&sc->lock));
- if ( !(sc->flags & FWRITE) )
+ if (!(sc->flags & FWRITE))
return;
for (i = 0; i < sc->nmidi; i++) {
md = sc->devs[i];
@@ -1420,7 +1433,6 @@ midiseq_open(int unit, int flags)
sc = device_lookup_private(&midi_cd, unit);
md = kmem_zalloc(sizeof(*md), KM_SLEEP);
- md->msc = sc;
md->unit = unit;
md->name = mi.name;
md->subtype = 0;
@@ -1451,8 +1463,8 @@ midiseq_reset(struct midi_dev *md)
static int
midiseq_out(struct midi_dev *md, u_char *bf, u_int cc, int chk)
{
- DPRINTFN(5, ("midiseq_out: m=%p, unit=%d, bf[0]=0x%02x, cc=%d\n",
- md->msc, md->unit, bf[0], cc));
+ DPRINTFN(5, ("midiseq_out: md=%p, unit=%d, bf[0]=0x%02x, cc=%d\n",
+ md, md->unit, bf[0], cc));
/* midi(4) does running status compression where appropriate. */
return midi_writebytes(md->unit, bf, cc);
Index: src/sys/dev/sequencervar.h
diff -u src/sys/dev/sequencervar.h:1.16 src/sys/dev/sequencervar.h:1.17
--- src/sys/dev/sequencervar.h:1.16 Sat Apr 27 22:12:42 2013
+++ src/sys/dev/sequencervar.h Mon Dec 22 07:02:22 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: sequencervar.h,v 1.16 2013/04/27 22:12:42 christos Exp $ */
+/* $NetBSD: sequencervar.h,v 1.17 2014/12/22 07:02:22 mrg Exp $ */
/*
* Copyright (c) 1998, 2008 The NetBSD Foundation, Inc.
@@ -68,7 +68,6 @@ struct midi_dev {
int instr_bank_size;
int unit;
struct sequencer_softc *seq;
- struct midi_softc *msc;
char doingsysex; /* doing a SEQ_SYSEX */
vnode_t *vp;
};
