Source-Changes-HG archive

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

[src/trunk]: src/sys/dev/audio Split sc_lock and sc_exlock.



details:   https://anonhg.NetBSD.org/src/rev/2253ba6d59b1
branches:  trunk
changeset: 1007983:2253ba6d59b1
user:      isaki <isaki%NetBSD.org@localhost>
date:      Sat Mar 07 06:25:57 2020 +0000

description:
Split sc_lock and sc_exlock.
Most (probably all) malloc/free (or routines which may sleep) now can be
called without holding mutex.
Pointed out by riastradh@.

diffstat:

 sys/dev/audio/audio.c    |  363 +++++++++++++++++++++++++++-------------------
 sys/dev/audio/audiovar.h |   17 +-
 2 files changed, 224 insertions(+), 156 deletions(-)

diffs (truncated from 1323 to 300 lines):

diff -r 0e092108e60d -r 2253ba6d59b1 sys/dev/audio/audio.c
--- a/sys/dev/audio/audio.c     Sat Mar 07 00:57:31 2020 +0000
+++ b/sys/dev/audio/audio.c     Sat Mar 07 06:25:57 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: audio.c,v 1.62 2020/03/04 14:19:41 isaki Exp $ */
+/*     $NetBSD: audio.c,v 1.63 2020/03/07 06:25:57 isaki Exp $ */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -142,7 +142,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.62 2020/03/04 14:19:41 isaki Exp $");
+__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.63 2020/03/07 06:25:57 isaki Exp $");
 
 #ifdef _KERNEL_OPT
 #include "audio.h"
@@ -497,8 +497,10 @@
 static void audio_softintr_rd(void *);
 static void audio_softintr_wr(void *);
 
-static int  audio_enter_exclusive(struct audio_softc *);
-static void audio_exit_exclusive(struct audio_softc *);
+static int audio_exlock_mutex_enter(struct audio_softc *);
+static void audio_exlock_mutex_exit(struct audio_softc *);
+static int audio_exlock_enter(struct audio_softc *);
+static void audio_exlock_exit(struct audio_softc *);
 static struct audio_softc *audio_file_enter(audio_file_t *, struct psref *);
 static void audio_file_exit(struct audio_softc *, struct psref *);
 static int audio_track_waitio(struct audio_softc *, audio_track_t *);
@@ -898,6 +900,7 @@
        sc->hw_hdl = hdlp;
        sc->hw_dev = parent;
 
+       sc->sc_exlock = 1;
        sc->sc_blk_ms = AUDIO_BLK_MS;
        SLIST_INIT(&sc->sc_files);
        cv_init(&sc->sc_exlockcv, "audiolk");
@@ -981,10 +984,8 @@
 
        /* Init hardware. */
        /* hw_probe() also validates [pr]hwfmt.  */
-       mutex_enter(sc->sc_lock);
        error = audio_hw_set_format(sc, mode, &phwfmt, &rhwfmt, &pfil, &rfil);
        if (error) {
-               mutex_exit(sc->sc_lock);
                aprint_error_dev(self, "audio_hw_set_format failed, "
                    "error = %d\n", error);
                goto bad;
@@ -995,7 +996,6 @@
         * attach time, we assume a success.
         */
        error = audio_mixers_init(sc, mode, &phwfmt, &rhwfmt, &pfil, &rfil);
-       mutex_exit(sc->sc_lock);
        if (sc->sc_pmixer == NULL && sc->sc_rmixer == NULL) {
                aprint_error_dev(self, "audio_mixers_init failed, "
                    "error = %d\n", error);
@@ -1087,11 +1087,13 @@
 #endif
 
        audiorescan(self, "audio", NULL);
+       sc->sc_exlock = 0;
        return;
 
 bad:
        /* Clearing hw_if means that device is attached but disabled. */
        sc->hw_if = NULL;
+       sc->sc_exlock = 0;
        aprint_error_dev(sc->sc_dev, "disabled\n");
        return;
 }
@@ -1309,6 +1311,7 @@
         * that hold sc, and any new calls with files that were for sc will
         * fail.  Thus, we now have exclusive access to the softc.
         */
+       sc->sc_exlock = 1;
 
        /*
         * Nuke all open instances.
@@ -1334,7 +1337,6 @@
        pmf_device_deregister(self);
 
        /* Free resources */
-       mutex_enter(sc->sc_lock);
        if (sc->sc_pmixer) {
                audio_mixer_destroy(sc, sc->sc_pmixer);
                kmem_free(sc->sc_pmixer, sizeof(*sc->sc_pmixer));
@@ -1343,7 +1345,6 @@
                audio_mixer_destroy(sc, sc->sc_rmixer);
                kmem_free(sc->sc_rmixer, sizeof(*sc->sc_rmixer));
        }
-       mutex_exit(sc->sc_lock);
        if (sc->sc_am)
                kern_free(sc->sc_am);
 
@@ -1415,12 +1416,12 @@
 }
 
 /*
- * Acquire sc_lock and enter exlock critical section.
- * If successful, it returns 0.  Otherwise returns errno.
+ * Enter critical section and also keep sc_lock.
+ * If successful, returns 0 with sc_lock held.  Otherwise returns errno.
  * Must be called without sc_lock held.
  */
 static int
-audio_enter_exclusive(struct audio_softc *sc)
+audio_exlock_mutex_enter(struct audio_softc *sc)
 {
        int error;
 
@@ -1446,23 +1447,51 @@
 }
 
 /*
- * Leave exlock critical section and release sc_lock.
+ * Exit critical section and exit sc_lock.
  * Must be called with sc_lock held.
  */
 static void
-audio_exit_exclusive(struct audio_softc *sc)
+audio_exlock_mutex_exit(struct audio_softc *sc)
 {
 
        KASSERT(mutex_owned(sc->sc_lock));
-       KASSERT(sc->sc_exlock);
-
-       /* Leave critical section */
+
        sc->sc_exlock = 0;
        cv_broadcast(&sc->sc_exlockcv);
        mutex_exit(sc->sc_lock);
 }
 
 /*
+ * Enter critical section.
+ * If successful, it returns 0.  Otherwise returns errno.
+ * Must be called without sc_lock held.
+ * This function returns without sc_lock held.
+ */
+static int
+audio_exlock_enter(struct audio_softc *sc)
+{
+       int error;
+
+       error = audio_exlock_mutex_enter(sc);
+       if (error)
+               return error;
+       mutex_exit(sc->sc_lock);
+       return 0;
+}
+
+/*
+ * Exit critical section.
+ * Must be called without sc_lock held.
+ */
+static void
+audio_exlock_exit(struct audio_softc *sc)
+{
+
+       mutex_enter(sc->sc_lock);
+       audio_exlock_mutex_exit(sc);
+}
+
+/*
  * Acquire sc from file, and increment the psref count.
  * If successful, returns sc.  Otherwise returns NULL.
  */
@@ -1576,7 +1605,7 @@
        if (sc == NULL || sc->hw_if == NULL)
                return ENXIO;
 
-       error = audio_enter_exclusive(sc);
+       error = audio_exlock_enter(sc);
        if (error)
                return error;
 
@@ -1596,7 +1625,7 @@
                error = ENXIO;
                break;
        }
-       audio_exit_exclusive(sc);
+       audio_exlock_exit(sc);
 
        return error;
 }
@@ -1930,14 +1959,14 @@
        if (sc == NULL || sc->hw_if == NULL)
                return ENXIO;
 
-       error = audio_enter_exclusive(sc);
+       error = audio_exlock_enter(sc);
        if (error)
                return error;
 
        device_active(sc->sc_dev, DVA_SYSTEM);
        error = audio_open(dev, sc, FWRITE, 0, curlwp, filep);
 
-       audio_exit_exclusive(sc);
+       audio_exlock_exit(sc);
        return error;
 }
 
@@ -1980,11 +2009,11 @@
        AUDIO_INITINFO(&ai);
        ai.play.sample_rate = sample_rate;
 
-       error = audio_enter_exclusive(sc);
+       error = audio_exlock_enter(sc);
        if (error)
                goto done;
        error = audio_file_setinfo(sc, file, &ai);
-       audio_exit_exclusive(sc);
+       audio_exlock_exit(sc);
 
 done:
        audio_file_exit(sc, &sc_ref);
@@ -2013,6 +2042,10 @@
 /*
  * Audio driver
  */
+
+/*
+ * Must be called with sc_exlock held and without sc_lock held.
+ */
 int
 audio_open(dev_t dev, struct audio_softc *sc, int flags, int ifmt,
        struct lwp *l, audio_file_t **bellfile)
@@ -2025,7 +2058,6 @@
        int fd;
        int error;
 
-       KASSERT(mutex_owned(sc->sc_lock));
        KASSERT(sc->sc_exlock);
 
        TRACE(1, "%sdev=%s flags=0x%x po=%d ro=%d",
@@ -2147,9 +2179,11 @@
                                        hwflags |= FREAD;
                        }
 
+                       mutex_enter(sc->sc_lock);
                        mutex_enter(sc->sc_intr_lock);
                        error = sc->hw_if->open(sc->hw_hdl, hwflags);
                        mutex_exit(sc->sc_intr_lock);
+                       mutex_exit(sc->sc_lock);
                        if (error)
                                goto bad2;
                }
@@ -2166,9 +2200,11 @@
                                } else {
                                        on = 0;
                                }
+                               mutex_enter(sc->sc_lock);
                                mutex_enter(sc->sc_intr_lock);
                                error = sc->hw_if->speaker_ctl(sc->hw_hdl, on);
                                mutex_exit(sc->sc_intr_lock);
+                               mutex_exit(sc->sc_lock);
                                if (error)
                                        goto bad3;
                        }
@@ -2185,12 +2221,14 @@
        if (af->ptrack && sc->sc_popens == 0) {
                if (sc->hw_if->init_output) {
                        hwbuf = &sc->sc_pmixer->hwbuf;
+                       mutex_enter(sc->sc_lock);
                        mutex_enter(sc->sc_intr_lock);
                        error = sc->hw_if->init_output(sc->hw_hdl,
                            hwbuf->mem,
                            hwbuf->capacity *
                            hwbuf->fmt.channels * hwbuf->fmt.stride / NBBY);
                        mutex_exit(sc->sc_intr_lock);
+                       mutex_exit(sc->sc_lock);
                        if (error)
                                goto bad3;
                }
@@ -2199,12 +2237,14 @@
        if (af->rtrack && sc->sc_ropens == 0) {
                if (sc->hw_if->init_input) {
                        hwbuf = &sc->sc_rmixer->hwbuf;
+                       mutex_enter(sc->sc_lock);
                        mutex_enter(sc->sc_intr_lock);
                        error = sc->hw_if->init_input(sc->hw_hdl,
                            hwbuf->mem,
                            hwbuf->capacity *
                            hwbuf->fmt.channels * hwbuf->fmt.stride / NBBY);
                        mutex_exit(sc->sc_intr_lock);
+                       mutex_exit(sc->sc_lock);
                        if (error)
                                goto bad3;
                }
@@ -2220,6 +2260,7 @@
         * Count up finally.
         * Don't fail from here.
         */
+       mutex_enter(sc->sc_lock);
        if (af->ptrack)
                sc->sc_popens++;
        if (af->rtrack)
@@ -2227,6 +2268,7 @@



Home | Main Index | Thread Index | Old Index