Source-Changes-HG archive

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

[src/trunk]: src/sys/dev/audio Remove unnecessary file lock.



details:   https://anonhg.NetBSD.org/src/rev/40da152ac2a4
branches:  trunk
changeset: 451448:40da152ac2a4
user:      isaki <isaki%NetBSD.org@localhost>
date:      Thu May 23 12:20:27 2019 +0000

description:
Remove unnecessary file lock.
It has been introduced to prevent multiple syscalls entering
simultaneously.  But it's completely unnecessary.
It fixes firefox problem in PR kern/54177.

diffstat:

 sys/dev/audio/audio.c    |  140 +++++-----------------------------------------
 sys/dev/audio/audiodef.h |    9 +--
 2 files changed, 16 insertions(+), 133 deletions(-)

diffs (truncated from 421 to 300 lines):

diff -r 547b91c8213d -r 40da152ac2a4 sys/dev/audio/audio.c
--- a/sys/dev/audio/audio.c     Thu May 23 11:13:17 2019 +0000
+++ b/sys/dev/audio/audio.c     Thu May 23 12:20:27 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: audio.c,v 1.8 2019/05/21 12:52:57 isaki Exp $  */
+/*     $NetBSD: audio.c,v 1.9 2019/05/23 12:20:27 isaki Exp $  */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -131,14 +131,7 @@
  *   neither lock were necessary.  Currently, on the other hand, since
  *   these may be also called after attach, the thread lock is required.
  *
- * In addition, there are two additional locks.
- *
- * - file->lock.  This is a variable protected by sc_lock and is similar
- *   to the "thread lock".  This is one for each file.  If any thread
- *   context and software interrupt context who want to access the file
- *   structure, they must acquire this lock before.  It protects
- *   descriptor's consistency among multithreaded accesses.  Since this
- *   lock uses sc_lock, don't acquire from hardware interrupt context.
+ * In addition, there is an additional lock.
  *
  * - track->lock.  This is an atomic variable and is similar to the
  *   "interrupt lock".  This is one for each track.  If any thread context
@@ -149,7 +142,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.8 2019/05/21 12:52:57 isaki Exp $");
+__KERNEL_RCSID(0, "$NetBSD: audio.c,v 1.9 2019/05/23 12:20:27 isaki Exp $");
 
 #ifdef _KERNEL_OPT
 #include "audio.h"
@@ -507,8 +500,6 @@
 static int  audio_enter_exclusive(struct audio_softc *);
 static void audio_exit_exclusive(struct audio_softc *);
 static int audio_track_waitio(struct audio_softc *, audio_track_t *);
-static int audio_file_acquire(struct audio_softc *, audio_file_t *);
-static void audio_file_release(struct audio_softc *, audio_file_t *);
 
 static int audioclose(struct file *);
 static int audioread(struct file *, off_t *, struct uio *, kauth_cred_t, int);
@@ -1431,57 +1422,6 @@
 }
 
 /*
- * Acquire the file lock.
- * If file is acquired successfully, returns 0.  Otherwise returns errno.
- * In both case, sc_lock is released.
- */
-static int
-audio_file_acquire(struct audio_softc *sc, audio_file_t *file)
-{
-       int error;
-
-       KASSERT(!mutex_owned(sc->sc_lock));
-
-       mutex_enter(sc->sc_lock);
-       if (sc->sc_dying) {
-               mutex_exit(sc->sc_lock);
-               return EIO;
-       }
-
-       while (__predict_false(file->lock != 0)) {
-               error = cv_wait_sig(&sc->sc_exlockcv, sc->sc_lock);
-               if (sc->sc_dying)
-                       error = EIO;
-               if (error) {
-                       mutex_exit(sc->sc_lock);
-                       return error;
-               }
-       }
-
-       /* Mark this file locked */
-       file->lock = 1;
-       mutex_exit(sc->sc_lock);
-
-       return 0;
-}
-
-/*
- * Release the file lock.
- */
-static void
-audio_file_release(struct audio_softc *sc, audio_file_t *file)
-{
-
-       KASSERT(!mutex_owned(sc->sc_lock));
-
-       mutex_enter(sc->sc_lock);
-       KASSERT(file->lock);
-       file->lock = 0;
-       cv_broadcast(&sc->sc_exlockcv);
-       mutex_exit(sc->sc_lock);
-}
-
-/*
  * Try to acquire track lock.
  * It doesn't block if the track lock is already aquired.
  * Returns true if the track lock was acquired, or false if the track
@@ -1563,11 +1503,7 @@
        sc = file->sc;
        dev = file->dev;
 
-       /* Acquire file lock and exlock */
-       /* XXX what should I do when an error occurs? */
-       error = audio_file_acquire(sc, file);
-       if (error)
-               return error;
+       /* audio_{enter,exit}_exclusive() is called by lower audio_close() */
 
        device_active(sc->sc_dev, DVA_SYSTEM);
        switch (AUDIODEV(dev)) {
@@ -1590,11 +1526,6 @@
                fp->f_audioctx = NULL;
        }
 
-       /*
-        * Since file has already been destructed,
-        * audio_file_release() is not necessary.
-        */
-
        return error;
 }
 
@@ -1612,10 +1543,6 @@
        sc = file->sc;
        dev = file->dev;
 
-       error = audio_file_acquire(sc, file);
-       if (error)
-               return error;
-
        if (fp->f_flag & O_NONBLOCK)
                ioflag |= IO_NDELAY;
 
@@ -1632,7 +1559,6 @@
                error = ENXIO;
                break;
        }
-       audio_file_release(sc, file);
 
        return error;
 }
@@ -1651,10 +1577,6 @@
        sc = file->sc;
        dev = file->dev;
 
-       error = audio_file_acquire(sc, file);
-       if (error)
-               return error;
-
        if (fp->f_flag & O_NONBLOCK)
                ioflag |= IO_NDELAY;
 
@@ -1671,7 +1593,6 @@
                error = ENXIO;
                break;
        }
-       audio_file_release(sc, file);
 
        return error;
 }
@@ -1690,10 +1611,6 @@
        sc = file->sc;
        dev = file->dev;
 
-       error = audio_file_acquire(sc, file);
-       if (error)
-               return error;
-
        switch (AUDIODEV(dev)) {
        case SOUND_DEVICE:
        case AUDIO_DEVICE:
@@ -1714,7 +1631,6 @@
                error = ENXIO;
                break;
        }
-       audio_file_release(sc, file);
 
        return error;
 }
@@ -1750,9 +1666,6 @@
        sc = file->sc;
        dev = file->dev;
 
-       if (audio_file_acquire(sc, file) != 0)
-               return 0;
-
        switch (AUDIODEV(dev)) {
        case SOUND_DEVICE:
        case AUDIO_DEVICE:
@@ -1766,7 +1679,6 @@
                revents = POLLERR;
                break;
        }
-       audio_file_release(sc, file);
 
        return revents;
 }
@@ -1784,10 +1696,6 @@
        sc = file->sc;
        dev = file->dev;
 
-       error = audio_file_acquire(sc, file);
-       if (error)
-               return error;
-
        switch (AUDIODEV(dev)) {
        case SOUND_DEVICE:
        case AUDIO_DEVICE:
@@ -1801,7 +1709,6 @@
                error = ENXIO;
                break;
        }
-       audio_file_release(sc, file);
 
        return error;
 }
@@ -1820,10 +1727,6 @@
        sc = file->sc;
        dev = file->dev;
 
-       error = audio_file_acquire(sc, file);
-       if (error)
-               return error;
-
        mutex_enter(sc->sc_lock);
        device_active(sc->sc_dev, DVA_SYSTEM); /* XXXJDM */
        mutex_exit(sc->sc_lock);
@@ -1840,7 +1743,6 @@
                error = ENOTSUP;
                break;
        }
-       audio_file_release(sc, file);
 
        return error;
 }
@@ -1887,11 +1789,6 @@
 
        sc = file->sc;
 
-       /* XXX what should I do when an error occurs? */
-       error = audio_file_acquire(sc, file);
-       if (error)
-               return error;
-
        device_active(sc->sc_dev, DVA_SYSTEM);
        error = audio_close(sc, file);
 
@@ -1911,13 +1808,7 @@
        int error;
 
        sc = file->sc;
-       error = audio_file_acquire(sc, file);
-       if (error)
-               return error;
-
        error = audio_write(sc, uio, 0, file);
-
-       audio_file_release(sc, file);
        return error;
 }
 
@@ -2179,6 +2070,9 @@
        return error;
 }
 
+/*
+ * Must NOT called with sc_lock nor sc_exlock held.
+ */
 int
 audio_close(struct audio_softc *sc, audio_file_t *file)
 {
@@ -2186,7 +2080,6 @@
        int error;
 
        KASSERT(!mutex_owned(sc->sc_lock));
-       KASSERT(file->lock);
 
        TRACEF(1, file, "%spid=%d.%d po=%d ro=%d",
            (audiodebug >= 3) ? "start " : "",
@@ -2209,10 +2102,8 @@
        /* Then, acquire exclusive lock to protect counters. */
        /* XXX what should I do when an error occurs? */
        error = audio_enter_exclusive(sc);
-       if (error) {
-               audio_file_release(sc, file);
+       if (error)
                return error;
-       }
 
        if (file->ptrack) {
                /* Call hw halt_output if this is the last playback track. */
@@ -2294,7 +2185,6 @@
        TRACET(2, track, "resid=%zd", uio->uio_resid);
 
        KASSERT(!mutex_owned(sc->sc_lock));
-       KASSERT(file->lock);



Home | Main Index | Thread Index | Old Index