Source-Changes-HG archive

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

[src/trunk]: src/sys/dev/audio Protect also audioopen() and audiobellopen() f...



details:   https://anonhg.NetBSD.org/src/rev/382cb86df004
branches:  trunk
changeset: 959354:382cb86df004
user:      isaki <isaki%NetBSD.org@localhost>
date:      Tue Feb 09 12:36:34 2021 +0000

description:
Protect also audioopen() and audiobellopen() from audiodetach() with
psref(9), as well as others(audioread, audiowrite, etc..).
- Rename audio_file_enter to audio_sc_acquire_fromfile, audio_file_exit
  to audio_sc_release, for clarify.  These are the reference counter for
  this sc.
- Introduce audio_sc_acquire_foropen for audio{,bell}open.
- audio_open needs to examine sc_dying again before inserting it into
  sc_files, in order to keep sc_files consistency.
The race between audiodetach and audioopen is pointed out by riastradh@.
Thank you for many advices.

diffstat:

 sys/dev/audio/audio.c |  181 +++++++++++++++++++++++++++++++++++--------------
 1 files changed, 127 insertions(+), 54 deletions(-)

diffs (truncated from 450 to 300 lines):

diff -r 50378a1f9c76 -r 382cb86df004 sys/dev/audio/audio.c
--- a/sys/dev/audio/audio.c     Tue Feb 09 11:57:20 2021 +0000
+++ b/sys/dev/audio/audio.c     Tue Feb 09 12:36:34 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: audio.c,v 1.89 2021/02/09 05:53:14 isaki Exp $ */
+/*     $NetBSD: audio.c,v 1.90 2021/02/09 12:36:34 isaki Exp $ */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -138,7 +138,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.89 2021/02/09 05:53:14 isaki Exp $");
+__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.90 2021/02/09 12:36:34 isaki Exp $");
 
 #ifdef _KERNEL_OPT
 #include "audio.h"
@@ -521,8 +521,10 @@
 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 void audio_sc_acquire_foropen(struct audio_softc *, struct psref *);
+static struct audio_softc *audio_sc_acquire_fromfile(audio_file_t *,
+       struct psref *);
+static void audio_sc_release(struct audio_softc *, struct psref *);
 static int audio_track_waitio(struct audio_softc *, audio_track_t *);
 
 static int audioclose(struct file *);
@@ -1302,7 +1304,10 @@
        if (error)
                return error;
 
-       /* delete sysctl nodes */
+       /*
+        * This waits currently running sysctls to finish if exists.
+        * After this, no more new sysctls will come.
+        */
        sysctl_teardown(&sc->sc_log);
 
        mutex_enter(sc->sc_lock);
@@ -1531,11 +1536,40 @@
 }
 
 /*
- * Acquire sc from file, and increment the psref count.
+ * Increment reference counter for this sc.
+ * This is intended to be used for open.
+ */
+void
+audio_sc_acquire_foropen(struct audio_softc *sc, struct psref *refp)
+{
+       int s;
+
+       /* psref(9) forbids to migrate CPUs */
+       curlwp_bind();
+
+       /* Block audiodetach while we acquire a reference */
+       s = pserialize_read_enter();
+
+       /*
+        * We don't examine sc_dying here.  However, all open methods
+        * call audio_exlock_enter() right after this, so we can examine
+        * sc_dying in it.
+        */
+
+       /* Acquire a reference */
+       psref_acquire(refp, &sc->sc_psref, audio_psref_class);
+
+       /* Now sc won't go away until we drop the reference count */
+       pserialize_read_exit(s);
+}
+
+/*
+ * Get sc from file, and increment reference counter for this sc.
+ * This is intended to be used for methods other than open.
  * If successful, returns sc.  Otherwise returns NULL.
  */
 struct audio_softc *
-audio_file_enter(audio_file_t *file, struct psref *refp)
+audio_sc_acquire_fromfile(audio_file_t *file, struct psref *refp)
 {
        int s;
        bool dying;
@@ -1563,10 +1597,10 @@
 }
 
 /*
- * Decrement the psref count.
+ * Decrement reference counter for this sc.
  */
 void
-audio_file_exit(struct audio_softc *sc, struct psref *refp)
+audio_sc_release(struct audio_softc *sc, struct psref *refp)
 {
 
        psref_release(refp, &sc->sc_psref, audio_psref_class);
@@ -1644,6 +1678,7 @@
 audioopen(dev_t dev, int flags, int ifmt, struct lwp *l)
 {
        struct audio_softc *sc;
+       struct psref sc_ref;
        int error;
 
        /* Find the device */
@@ -1651,9 +1686,11 @@
        if (sc == NULL || sc->hw_if == NULL)
                return ENXIO;
 
+       audio_sc_acquire_foropen(sc, &sc_ref);
+
        error = audio_exlock_enter(sc);
        if (error)
-               return error;
+               goto done;
 
        device_active(sc->sc_dev, DVA_SYSTEM);
        switch (AUDIODEV(dev)) {
@@ -1673,6 +1710,8 @@
        }
        audio_exlock_exit(sc);
 
+done:
+       audio_sc_release(sc, &sc_ref);
        return error;
 }
 
@@ -1697,7 +1736,7 @@
         * - free all memory objects, regardless of sc.
         */
 
-       sc = audio_file_enter(file, &sc_ref);
+       sc = audio_sc_acquire_fromfile(file, &sc_ref);
        if (sc) {
                switch (AUDIODEV(dev)) {
                case SOUND_DEVICE:
@@ -1715,7 +1754,7 @@
                        break;
                }
 
-               audio_file_exit(sc, &sc_ref);
+               audio_sc_release(sc, &sc_ref);
        }
 
        /* Free memory objects anyway */
@@ -1744,7 +1783,7 @@
        file = fp->f_audioctx;
        dev = file->dev;
 
-       sc = audio_file_enter(file, &sc_ref);
+       sc = audio_sc_acquire_fromfile(file, &sc_ref);
        if (sc == NULL)
                return EIO;
 
@@ -1765,7 +1804,7 @@
                break;
        }
 
-       audio_file_exit(sc, &sc_ref);
+       audio_sc_release(sc, &sc_ref);
        return error;
 }
 
@@ -1783,7 +1822,7 @@
        file = fp->f_audioctx;
        dev = file->dev;
 
-       sc = audio_file_enter(file, &sc_ref);
+       sc = audio_sc_acquire_fromfile(file, &sc_ref);
        if (sc == NULL)
                return EIO;
 
@@ -1804,7 +1843,7 @@
                break;
        }
 
-       audio_file_exit(sc, &sc_ref);
+       audio_sc_release(sc, &sc_ref);
        return error;
 }
 
@@ -1822,7 +1861,7 @@
        file = fp->f_audioctx;
        dev = file->dev;
 
-       sc = audio_file_enter(file, &sc_ref);
+       sc = audio_sc_acquire_fromfile(file, &sc_ref);
        if (sc == NULL)
                return EIO;
 
@@ -1847,7 +1886,7 @@
                break;
        }
 
-       audio_file_exit(sc, &sc_ref);
+       audio_sc_release(sc, &sc_ref);
        return error;
 }
 
@@ -1861,7 +1900,7 @@
        KASSERT(fp->f_audioctx);
        file = fp->f_audioctx;
 
-       sc = audio_file_enter(file, &sc_ref);
+       sc = audio_sc_acquire_fromfile(file, &sc_ref);
        if (sc == NULL)
                return EIO;
 
@@ -1872,7 +1911,7 @@
        st->st_gid = kauth_cred_getegid(fp->f_cred);
        st->st_mode = S_IFCHR;
 
-       audio_file_exit(sc, &sc_ref);
+       audio_sc_release(sc, &sc_ref);
        return 0;
 }
 
@@ -1890,7 +1929,7 @@
        file = fp->f_audioctx;
        dev = file->dev;
 
-       sc = audio_file_enter(file, &sc_ref);
+       sc = audio_sc_acquire_fromfile(file, &sc_ref);
        if (sc == NULL)
                return POLLERR;
 
@@ -1908,7 +1947,7 @@
                break;
        }
 
-       audio_file_exit(sc, &sc_ref);
+       audio_sc_release(sc, &sc_ref);
        return revents;
 }
 
@@ -1925,7 +1964,7 @@
        file = fp->f_audioctx;
        dev = file->dev;
 
-       sc = audio_file_enter(file, &sc_ref);
+       sc = audio_sc_acquire_fromfile(file, &sc_ref);
        if (sc == NULL)
                return EIO;
 
@@ -1943,7 +1982,7 @@
                break;
        }
 
-       audio_file_exit(sc, &sc_ref);
+       audio_sc_release(sc, &sc_ref);
        return error;
 }
 
@@ -1961,7 +2000,7 @@
        file = fp->f_audioctx;
        dev = file->dev;
 
-       sc = audio_file_enter(file, &sc_ref);
+       sc = audio_sc_acquire_fromfile(file, &sc_ref);
        if (sc == NULL)
                return EIO;
 
@@ -1982,7 +2021,7 @@
                break;
        }
 
-       audio_file_exit(sc, &sc_ref);
+       audio_sc_release(sc, &sc_ref);
        return error;
 }
 
@@ -1998,6 +2037,7 @@
 audiobellopen(dev_t dev, audio_file_t **filep)
 {
        struct audio_softc *sc;
+       struct psref sc_ref;
        int error;
 
        /* Find the device */
@@ -2005,14 +2045,18 @@
        if (sc == NULL || sc->hw_if == NULL)
                return ENXIO;
 
+       audio_sc_acquire_foropen(sc, &sc_ref);
+
        error = audio_exlock_enter(sc);
        if (error)
-               return error;
+               goto done;
 
        device_active(sc->sc_dev, DVA_SYSTEM);
        error = audio_open(dev, sc, FWRITE, 0, curlwp, filep);
 
        audio_exlock_exit(sc);
+done:
+       audio_sc_release(sc, &sc_ref);
        return error;
 }



Home | Main Index | Thread Index | Old Index