From: ratchov Date: Sat, 16 May 2015 09:56:10 +0000 (+0000) Subject: Use device_lookup() instead of digging into midi_cd.cd_devs[] and X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d0b9e3b5a0b177e4043bc1d0513335aa84dc2107;p=openbsd 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 --- 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;