Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/netbsd-7]: src/sys/dev Pull up following revision(s) (requested by mrg i...



details:   https://anonhg.NetBSD.org/src/rev/fd677a14a4b4
branches:  netbsd-7
changeset: 798821:fd677a14a4b4
user:      martin <martin%NetBSD.org@localhost>
date:      Sun Jan 11 14:13:25 2015 +0000

description:
Pull up following revision(s) (requested by mrg in ticket #407):
        sys/dev/midivar.h: revision 1.20
        sys/dev/usb/umidivar.h: file removal
        sys/dev/midi.c: revision 1.82
        sys/dev/midi.c: revision 1.83
        sys/dev/usb/FILES: revision 1.13
        sys/dev/midi_if.h: revision 1.26
        sys/dev/sequencer.c: revision 1.60
        sys/dev/sequencer.c: revision 1.61
        sys/dev/sequencer.c: revision 1.62
        sys/dev/sequencer.c: revision 1.63
        sys/dev/usb/umidi_quirks.c: revision 1.19
        sys/dev/usb/umidi.c: revision 1.66
        sys/dev/usb/umidi.c: revision 1.67
        sys/dev/usb/umidi.c: revision 1.68
        sys/dev/usb/umidireg.h: file removal
        sys/dev/sequencervar.h: revision 1.17
fix the midi_if documentation to properly describe the locks that will
be held during various operations.
various umidi clean ups:
- move the contents of umidi{reg,var}.h into umidi.c directly as they
  are not referenced by any other file.
- remove the useless include of umidi{reg,var}.h from umidi_quirks.c.
- add reference counting and wait/broadcast support to the IO paths.
- fix the error handling in midi_attach() and midi_open().
- sprinkle KASSERT() in several places.
- drop the local interrupt lock before calling into various parts of
  the USB code.  fixes lockdebug issues, and likely hangs.
- rename "binded" member as "bound".
with these most of the panics and problems i've seen are gone.  there
is still one lockdebug panic to deal with that happens when unplugging
umidi while midiplay(1) is running.
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.
use __func__ in some debug messages.
- check sc->dying after sleeping in several more places, and
  convert it into EIO error where necessary.
- remove a wrong additional mutex_exit() call.
- make sure to check sc->dying under the device lock.
- fix a confusion between midi(4) unit and connected to sequencer
  devices.
- minor comment/debug clean ups.
fixes problems attempting to read or write from the right midi(4)
device using the sequencer(4) device when one or more of the
non-final devices fails to open with midiseq_open().
fix !AUDIO_DEBUG build.
CID/1261465: Dereference after NULL check.
CID/1261467: Unreachable code
actually fix one of the previous:  don't test for NULL after deref.

diffstat:

 sys/dev/midi.c             |  138 ++++++++++++++---
 sys/dev/midi_if.h          |   12 +-
 sys/dev/midivar.h          |    4 +-
 sys/dev/sequencer.c        |   80 ++++++---
 sys/dev/sequencervar.h     |    3 +-
 sys/dev/usb/FILES          |    2 -
 sys/dev/usb/umidi.c        |  349 +++++++++++++++++++++++++++++++++++++-------
 sys/dev/usb/umidi_quirks.c |    6 +-
 sys/dev/usb/umidireg.h     |   79 ----------
 sys/dev/usb/umidivar.h     |  129 ----------------
 10 files changed, 467 insertions(+), 335 deletions(-)

diffs (truncated from 1672 to 300 lines):

diff -r 51c19c8e797c -r fd677a14a4b4 sys/dev/midi.c
--- a/sys/dev/midi.c    Sun Jan 11 13:05:13 2015 +0000
+++ b/sys/dev/midi.c    Sun Jan 11 14:13:25 2015 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: midi.c,v 1.81 2014/07/25 08:10:35 dholland Exp $       */
+/*     $NetBSD: midi.c,v 1.81.2.1 2015/01/11 14:13:25 martin 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.81.2.1 2015/01/11 14:13:25 martin 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);
@@ -864,6 +880,8 @@
        mutex_enter(sc->lock);
        /* midi_start_output(sc); anything buffered => pbus already set! */
        while (sc->pbus) {
+               if (sc->dying)
+                       break;
                DPRINTFN(8,("midiclose sleep ...\n"));
                cv_wait(&sc->wchan, sc->lock);
        }
@@ -952,6 +970,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 +983,10 @@
                        mutex_enter(sc->lock);
                        if (error)
                                break;
+                       if (sc->dying) {
+                               error = EIO;
+                               break;
+                       }
                        buf_cur += appetite;
                }
                
@@ -1253,13 +1279,13 @@
                error = sc->hw_if->output(sc->hw_hdl, *buf_cur);
                if (error &&  error != EINPROGRESS)
                        break;
-               ++ buf_cur;
+               ++buf_cur;
                MIDI_BUF_WRAP(buf);
-               -- msglen;
+               --msglen;
                if (msglen)
                        *idx_cur = PACK_MB_IDX(MB_IDX_CAT(*idx_cur),msglen);
                else {
-                       ++ idx_cur;
+                       ++idx_cur;
                        MIDI_BUF_WRAP(idx);
                }
                if (!error) {
@@ -1306,9 +1332,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,19 +1361,17 @@
 
        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);
-               ++ ibuf;
+               ++ibuf;
                switch ( got) {
                case FST_MORE:
                        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 +1396,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 +1426,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 +1461,9 @@
                mutex_exit(sc->lock);
                return EIO;
        }
+
+       sc->refcnt++;
+
        mb = &sc->outbuf;
        error = 0;
        while (uio->uio_resid > 0 && !error) {
@@ -1444,8 +1486,8 @@
                                 * the common syscall code will automagically
                                 * convert this to success with a short count.
                                 */
-                               mutex_exit(sc->lock);
-                               return EWOULDBLOCK;
+                               error = EWOULDBLOCK;
+                               goto out;
                        }
                        if (pollout) {
                                mutex_exit(sc->lock);
@@ -1454,14 +1496,15 @@
                                pollout = 0;
                        } else
                                error = cv_wait_sig(&sc->wchan, sc->lock);
+                       if (sc->dying)
+                               error = EIO;
                        if (error) {
                                /*
                                 * Similarly, the common code will handle
                                 * EINTR and ERESTART properly here, changing to
                                 * a short count if something transferred.
                                 */
-                               mutex_exit(sc->lock);
-                               return error;
+                               goto out;
                        }
                }
 
@@ -1492,7 +1535,7 @@
                               "xfrcount=%zu inp=%p\n",
                               error, xfrcount, inp);
 #endif
-               if ( error )
+               if (error)
                        break;
                
                /*
@@ -1514,6 +1557,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,11 +1577,17 @@
            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]));
 
        mutex_enter(sc->lock);
-       error = real_writebytes(sc, bf, cc);
+       if (sc->dying)
+               error = EIO;
+       else
+               error = real_writebytes(sc, bf, cc);
        mutex_exit(sc->lock);
 
        return error;
@@ -1548,12 +1602,18 @@
        MIDI_BUF_DECLARE(buf);
 
        (void)buf_end;
-       sc = device_lookup_private(&midi_cd, MIDIUNIT(dev));;
-       if (sc->dying)
+       sc = device_lookup_private(&midi_cd, MIDIUNIT(dev));
+
+       mutex_enter(sc->lock);
+       if (sc->dying) {
+               mutex_exit(sc->lock);
                return EIO;
+       }
        hw = sc->hw_if;
        error = 0;
 
+       sc->refcnt++;
+
        DPRINTFN(5,("midiioctl: %p cmd=0x%08lx\n", sc, cmd));
 
        switch (cmd) {
@@ -1574,13 +1634,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 +1653,7 @@
                        sc->async = 0;
                }
                mutex_exit(proc_lock);
+               mutex_enter(sc->lock);
                break;
 
 #if 0
@@ -1607,20 +1667,24 @@



Home | Main Index | Thread Index | Old Index