Use device_lookup() instead of digging into midi_cd.cd_devs[] and

maintaining a "dying" flag which is already present in the device
structure. As a side-effect, this adds the missing refcounting
that mididetach() was missing. With from mpi@ and dlg@

ok mpi

2 files changed, 134 insertions, 96 deletions

diff --git a/sys/dev/midi.c b/sys/dev/midi.c
index e83997e014b..16235f24225 100644
--- a/sys/dev/midi.c
+++ b/sys/dev/midi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: midi.c,v 1.38 2015/05/12 18:39:30 ratchov Exp $	*/
+/*	$OpenBSD: midi.c,v 1.39 2015/05/16 09:56:10 ratchov Exp $	*/
 
 /*
  * Copyright (c) 2003, 2004 Alexandre Ratchov
@@ -84,7 +84,8 @@ midi_iintr(void *addr, int data)
 	struct midi_softc  *sc = (struct midi_softc *)addr;
 	struct midi_buffer *mb = &sc->inbuf;
 
-	if (sc->isdying || !(sc->flags & FREAD))
+	MUTEX_ASSERT_LOCKED(&audio_lock);
+	if (!(sc->dev.dv_flags & DVF_ACTIVE) || !(sc->flags & FREAD))
 		return;
 
 	if (MIDIBUF_ISFULL(mb))
@@ -105,31 +106,35 @@ midi_iintr(void *addr, int data)
 int
 midiread(dev_t dev, struct uio *uio, int ioflag)
 {
-	struct midi_softc  *sc = MIDI_DEV2SC(dev);
+	struct midi_softc *sc;
 	struct midi_buffer *mb = &sc->inbuf;
 	size_t count;
 	int error;
 
-	if (!(sc->flags & FREAD))
+	sc = (struct midi_softc *)device_lookup(&midi_cd, minor(dev));
+	if (sc == NULL)
 		return ENXIO;
+	if (!(sc->flags & FREAD)) {
+		error = ENXIO;
+		goto done;
+	}
 
 	/* if there is no data then sleep (unless IO_NDELAY flag is set) */
-
+	error = 0;
 	mtx_enter(&audio_lock);
 	while (MIDIBUF_ISEMPTY(mb)) {
-		if (sc->isdying) {
-			mtx_leave(&audio_lock);
-			return EIO;
-		}
 		if (ioflag & IO_NDELAY) {
 			mtx_leave(&audio_lock);
-			return EWOULDBLOCK;
+			error = EWOULDBLOCK;
+			goto done;
 		}
 		sc->rchan = 1;
 		error = msleep(&sc->rchan, &audio_lock, PWAIT | PCATCH, "mid_rd", 0);
+		if (!(sc->dev.dv_flags & DVF_ACTIVE))
+			error = EIO;
 		if (error) {
 			mtx_leave(&audio_lock);
-			return error;
+			goto done;
 		}
 	}
 
@@ -144,33 +149,36 @@ midiread(dev_t dev, struct uio *uio, int ioflag)
 		mtx_leave(&audio_lock);
 		error = uiomove(mb->data + mb->start, count, uio);
 		if (error)
-			return error;
+			goto done;
 		mtx_enter(&audio_lock);
 		MIDIBUF_REMOVE(mb, count);
 	}
 	mtx_leave(&audio_lock);
-	return 0;
+done:
+	device_unref(&sc->dev);
+	return error;
 }
 
 void
 midi_ointr(void *addr)
 {
-	struct midi_softc  *sc = (struct midi_softc *)addr;
+	struct midi_softc *sc = (struct midi_softc *)addr;
 	struct midi_buffer *mb;
 
 	MUTEX_ASSERT_LOCKED(&audio_lock);
-	if (!(sc->flags & FWRITE) && !sc->isdying) {
-		mb = &sc->outbuf;
-		if (mb->used > 0) {
+	if (!(sc->dev.dv_flags & DVF_ACTIVE) || !(sc->flags & FWRITE))
+		return;
+	
+	mb = &sc->outbuf;
+	if (mb->used > 0) {
 #ifdef MIDI_DEBUG
-			if (!sc->isbusy) {
-				printf("midi_ointr: output must be busy\n");
-			}
+		if (!sc->isbusy) {
+			printf("midi_ointr: output must be busy\n");
+		}
 #endif
-			midi_out_do(sc);
-		} else if (sc->isbusy)
-			midi_out_stop(sc);
-	}
+		midi_out_do(sc);
+	} else if (sc->isbusy)
+		midi_out_stop(sc);
 }
 
 void
@@ -231,25 +239,30 @@ midi_out_do(struct midi_softc *sc)
 int
 midiwrite(dev_t dev, struct uio *uio, int ioflag)
 {
-	struct midi_softc  *sc = MIDI_DEV2SC(dev);
+	struct midi_softc *sc;
 	struct midi_buffer *mb = &sc->outbuf;
 	size_t count;
 	int error;
 
-	if (!(sc->flags & FWRITE))
+	sc = (struct midi_softc *)device_lookup(&midi_cd, minor(dev));
+	if (sc == NULL)
 		return ENXIO;
-	if (sc->isdying)
-		return EIO;
+	if (!(sc->flags & FWRITE)) {
+		error = ENXIO;
+		goto done;
+	}
 
 	/*
 	 * If IO_NDELAY flag is set then check if there is enough room
 	 * in the buffer to store at least one byte. If not then dont
 	 * start the write process.
 	 */
+	error = 0;
 	mtx_enter(&audio_lock);
 	if ((ioflag & IO_NDELAY) && MIDIBUF_ISFULL(mb) && (uio->uio_resid > 0)) {
 		mtx_leave(&audio_lock);
-		return EWOULDBLOCK;
+		error = EWOULDBLOCK;
+		goto done;
 	}
 
 	while (uio->uio_resid > 0) {
@@ -260,18 +273,16 @@ midiwrite(dev_t dev, struct uio *uio, int ioflag)
 				 * moved so we do not return EWOULDBLOCK
 				 */
 				mtx_leave(&audio_lock);
-				return 0;
+				goto done;
 			}
 			sc->wchan = 1;
 			error = msleep(&sc->wchan, &audio_lock,
 			    PWAIT | PCATCH, "mid_wr", 0);
+			if (!(sc->dev.dv_flags & DVF_ACTIVE))
+				error = EIO;
 			if (error) {
 				mtx_leave(&audio_lock);
-				return error;
-			}
-			if (sc->isdying) {
-				mtx_leave(&audio_lock);
-				return EIO;
+				goto done;
 			}
 		}
 
@@ -283,24 +294,26 @@ midiwrite(dev_t dev, struct uio *uio, int ioflag)
 		mtx_leave(&audio_lock);
 		error = uiomove(mb->data + MIDIBUF_END(mb), count, uio);
 		if (error)
-			return error;
+			goto done;
 		mtx_enter(&audio_lock);
 		mb->used += count;
 		midi_out_start(sc);
 	}
 	mtx_leave(&audio_lock);
-	return 0;
+done:
+	device_unref(&sc->dev);
+	return error;
 }
 
 int
 midipoll(dev_t dev, int events, struct proc *p)
 {
-	struct midi_softc *sc = MIDI_DEV2SC(dev);
+	struct midi_softc *sc;
 	int revents;
 
-	if (sc->isdying)
+	sc = (struct midi_softc *)device_lookup(&midi_cd, minor(dev));
+	if (sc == NULL)
 		return POLLERR;
-
 	revents = 0;
 	mtx_enter(&audio_lock);
 	if (events & (POLLIN | POLLRDNORM)) {
@@ -318,15 +331,21 @@ midipoll(dev_t dev, int events, struct proc *p)
 			selrecord(p, &sc->wsel);
 	}
 	mtx_leave(&audio_lock);
+	device_unref(&sc->dev);
 	return (revents);
 }
 
 int
 midikqfilter(dev_t dev, struct knote *kn)
 {
-	struct midi_softc *sc = MIDI_DEV2SC(dev);
+	struct midi_softc *sc;
 	struct klist 	  *klist;
+	int error;
 
+	sc = (struct midi_softc *)device_lookup(&midi_cd, minor(dev));
+	if (sc == NULL)
+		return ENXIO;
+	error = 0;
 	switch (kn->kn_filter) {
 	case EVFILT_READ:
 		klist = &sc->rsel.si_note;
@@ -337,15 +356,17 @@ midikqfilter(dev_t dev, struct knote *kn)
 		kn->kn_fop = &midiwrite_filtops;
 		break;
 	default:
-		return (EINVAL);
+		error = EINVAL;
+		goto done;
 	}
 	kn->kn_hook = (void *)sc;
 
 	mtx_enter(&audio_lock);
 	SLIST_INSERT_HEAD(klist, kn, kn_selnext);
 	mtx_leave(&audio_lock);
-
-	return (0);
+done:
+	device_unref(&sc->dev);
+	return error;
 }
 
 void
@@ -397,81 +418,90 @@ filt_midiwrite(struct knote *kn, long hint)
 int
 midiioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p)
 {
-	struct midi_softc *sc = MIDI_DEV2SC(dev);
-
-	if (sc->isdying)
-		return EIO;
+	struct midi_softc *sc;
+	int error;
 
+	sc = (struct midi_softc *)device_lookup(&midi_cd, minor(dev));
+	if (sc == NULL)
+		return ENXIO;
+	error = 0;
 	switch(cmd) {
 	case FIONBIO:
 		/* All handled in the upper FS layer */
 		break;
 	case FIOASYNC:
 		if (*(int *)addr) {
-			if (sc->async)
-				return EBUSY;
+			if (sc->async) {
+				error = EBUSY;
+				goto done;
+			}
 			sc->async = p;
 		} else
 			sc->async = 0;
 		break;
 	default:
-		return ENOTTY;
+		error = ENOTTY;
 	}
-	return 0;
+done:
+	device_unref(&sc->dev);
+	return error;
 }
 
 int
 midiopen(dev_t dev, int flags, int mode, struct proc *p)
 {
 	struct midi_softc *sc;
-	int err;
+	int error;
 
-	if (MIDI_UNIT(dev) >= midi_cd.cd_ndevs)
+	sc = (struct midi_softc *)device_lookup(&midi_cd, minor(dev));
+	if (sc == NULL)
 		return ENXIO;
-	sc = MIDI_DEV2SC(dev);
-	if (sc == NULL)		/* there may be more units than devices */
-		return ENXIO;
-	if (sc->isdying)
-		return EIO;
-	if (sc->flags)
-		return EBUSY;
-
+	error = 0;
+	if (sc->flags) {
+		error = EBUSY;
+		goto done;
+	}
 	MIDIBUF_INIT(&sc->inbuf);
 	MIDIBUF_INIT(&sc->outbuf);
 	sc->isbusy = 0;
 	sc->rchan = sc->wchan = 0;
 	sc->async = 0;
 	sc->flags = flags;
-	err = sc->hw_if->open(sc->hw_hdl, flags, midi_iintr, midi_ointr, sc);
-	if (err) {
+	error = sc->hw_if->open(sc->hw_hdl, flags, midi_iintr, midi_ointr, sc);
+	if (error)
 		sc->flags = 0;
-		return err;
-	}
-	return 0;
+done:
+	device_unref(&sc->dev);
+	return error;
 }
 
 int
 midiclose(dev_t dev, int fflag, int devtype, struct proc *p)
 {
-	struct midi_softc  *sc = MIDI_DEV2SC(dev);
+	struct midi_softc *sc;
 	struct midi_buffer *mb;
 	int error;
 
+	sc = (struct midi_softc *)device_lookup(&midi_cd, minor(dev));
+	if (sc == NULL)
+		return ENXIO;
+
+	/* start draining output buffer */
+	error = 0;
 	mb = &sc->outbuf;
-	if (!sc->isdying) {
-		/* start draining output buffer */
-		mtx_enter(&audio_lock);
-		if (!MIDIBUF_ISEMPTY(mb))
-			midi_out_start(sc);
-		while (sc->isbusy) {
-			sc->wchan = 1;
-			error = msleep(&sc->wchan, &audio_lock,
-			    PWAIT, "mid_dr", 5 * hz);
-			if (error || sc->isdying)
-				break;
-		}
-		mtx_leave(&audio_lock);
+	mtx_enter(&audio_lock);
+	if (!MIDIBUF_ISEMPTY(mb))
+		midi_out_start(sc);
+	while (sc->isbusy) {
+		sc->wchan = 1;
+		error = msleep(&sc->wchan, &audio_lock,
+		    PWAIT, "mid_dr", 5 * hz);
+		if (!(sc->dev.dv_flags & DVF_ACTIVE))
+			error = EIO;
+		if (error)
+			break;
 	}
+	mtx_leave(&audio_lock);
 
 	/*
 	 * some hw_if->close() reset immediately the midi uart
@@ -483,6 +513,7 @@ midiclose(dev_t dev, int fflag, int devtype, struct proc *p)
 	tsleep(&sc->wchan, PWAIT, "mid_cl", hz * MIDI_MAXWRITE / MIDI_RATE);
 	sc->hw_if->close(sc->hw_hdl);
 	sc->flags = 0;
+	device_unref(&sc->dev);
 	return 0;
 }
 
@@ -515,7 +546,6 @@ midiattach(struct device *parent, struct device *self, void *aux)
 #endif
 	sc->hw_if = hwif;
 	sc->hw_hdl = hdl;
-	sc->isdying = 0;
 	sc->hw_if->getinfo(sc->hw_hdl, &mi);
 	sc->props = mi.props;
 	sc->flags = 0;
@@ -529,16 +559,6 @@ mididetach(struct device *self, int flags)
 	struct midi_softc *sc = (struct midi_softc *)self;
 	int maj, mn;
 
-	sc->isdying = 1;
-	if (sc->wchan) {
-		sc->wchan = 0;
-		wakeup(&sc->wchan);
-	}
-	if (sc->rchan) {
-		sc->rchan = 0;
-		wakeup(&sc->rchan);
-	}
-
 	/* locate the major number */
 	for (maj = 0; maj < nchrdev; maj++) {
 		if (cdevsw[maj].d_open == midiopen) {
@@ -547,6 +567,27 @@ mididetach(struct device *self, int flags)
 			vdevgone(maj, mn, mn, VCHR);
 		}
 	}
+
+	/*
+	 * The close() method did nothing (device_lookup() returns
+	 * NULL), so quickly halt transfers (normally parent is already
+	 * gone, and code below is no-op), and wake-up user-land blocked
+	 * in read/write/ioctl, which return EIO.
+	 */
+	if (sc->flags) {
+		if (sc->flags & FREAD) {
+			sc->rchan = 0;
+			wakeup(&sc->rchan);
+			selwakeup(&sc->rsel);
+		}
+		if (sc->flags & FWRITE) {
+			sc->wchan = 0;
+			wakeup(&sc->wchan);
+			selwakeup(&sc->wsel);
+		}
+		sc->hw_if->close(sc->hw_hdl);
+		sc->flags = 0;
+	}
 	return 0;
 }
 
diff --git a/sys/dev/midivar.h b/sys/dev/midivar.h
index 48e61bdae4f..b195ab91acc 100644
--- a/sys/dev/midivar.h
+++ b/sys/dev/midivar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: midivar.h,v 1.8 2015/05/12 18:39:30 ratchov Exp $	*/
+/*	$OpenBSD: midivar.h,v 1.9 2015/05/16 09:56:10 ratchov Exp $	*/
 
 /*
  * Copyright (c) 2003, 2004 Alexandre Ratchov
@@ -27,8 +27,6 @@
 
 #define MIDI_MAXWRITE	32	/* max bytes to give to the uart at once */
 #define MIDI_RATE	3125	/* midi uart baud rate in bytes/second */
-#define MIDI_UNIT(a)	((a) & 0xff)
-#define MIDI_DEV2SC(a)	(midi_cd.cd_devs[MIDI_UNIT(a)])
 
 /*
  * simple ring buffer
@@ -73,7 +71,6 @@ struct midi_softc {
 	struct midi_hw_if  *hw_if;
 	void		   *hw_hdl;
 	int		    isbusy;		/* concerns only the output */
-	int		    isdying;
 	int		    flags;		/* open flags */
 	int		    props;		/* midi hw proprieties */
 	int		    rchan;