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.
This commit is contained in:
isaki
parent
fe6f1f01c1
commit
c173283618
|
|
@ -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 void audio_softintr_wr(void *);
|
|||
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);
|
||||
|
|
@ -1430,57 +1421,6 @@ audio_track_waitio(struct audio_softc *sc, audio_track_t *track)
|
|||
return error;
|
||||
}
|
||||
|
||||
/*
|
||||
* 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.
|
||||
|
|
@ -1563,11 +1503,7 @@ audioclose(struct file *fp)
|
|||
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 @@ audioclose(struct file *fp)
|
|||
fp->f_audioctx = NULL;
|
||||
}
|
||||
|
||||
/*
|
||||
* Since file has already been destructed,
|
||||
* audio_file_release() is not necessary.
|
||||
*/
|
||||
|
||||
return error;
|
||||
}
|
||||
|
||||
|
|
@ -1612,10 +1543,6 @@ audioread(struct file *fp, off_t *offp, struct uio *uio, kauth_cred_t cred,
|
|||
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 @@ audioread(struct file *fp, off_t *offp, struct uio *uio, kauth_cred_t cred,
|
|||
error = ENXIO;
|
||||
break;
|
||||
}
|
||||
audio_file_release(sc, file);
|
||||
|
||||
return error;
|
||||
}
|
||||
|
|
@ -1651,10 +1577,6 @@ audiowrite(struct file *fp, off_t *offp, struct uio *uio, kauth_cred_t cred,
|
|||
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 @@ audiowrite(struct file *fp, off_t *offp, struct uio *uio, kauth_cred_t cred,
|
|||
error = ENXIO;
|
||||
break;
|
||||
}
|
||||
audio_file_release(sc, file);
|
||||
|
||||
return error;
|
||||
}
|
||||
|
|
@ -1690,10 +1611,6 @@ audioioctl(struct file *fp, u_long cmd, void *addr)
|
|||
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 @@ audioioctl(struct file *fp, u_long cmd, void *addr)
|
|||
error = ENXIO;
|
||||
break;
|
||||
}
|
||||
audio_file_release(sc, file);
|
||||
|
||||
return error;
|
||||
}
|
||||
|
|
@ -1750,9 +1666,6 @@ audiopoll(struct file *fp, int events)
|
|||
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 @@ audiopoll(struct file *fp, int events)
|
|||
revents = POLLERR;
|
||||
break;
|
||||
}
|
||||
audio_file_release(sc, file);
|
||||
|
||||
return revents;
|
||||
}
|
||||
|
|
@ -1784,10 +1696,6 @@ audiokqfilter(struct file *fp, struct knote *kn)
|
|||
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 @@ audiokqfilter(struct file *fp, struct knote *kn)
|
|||
error = ENXIO;
|
||||
break;
|
||||
}
|
||||
audio_file_release(sc, file);
|
||||
|
||||
return error;
|
||||
}
|
||||
|
|
@ -1820,10 +1727,6 @@ audiommap(struct file *fp, off_t *offp, size_t len, int prot, int *flagsp,
|
|||
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 @@ audiommap(struct file *fp, off_t *offp, size_t len, int prot, int *flagsp,
|
|||
error = ENOTSUP;
|
||||
break;
|
||||
}
|
||||
audio_file_release(sc, file);
|
||||
|
||||
return error;
|
||||
}
|
||||
|
|
@ -1887,11 +1789,6 @@ audiobellclose(audio_file_t *file)
|
|||
|
||||
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 @@ audiobellwrite(audio_file_t *file, struct uio *uio)
|
|||
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 @@ bad1:
|
|||
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 @@ audio_close(struct audio_softc *sc, audio_file_t *file)
|
|||
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 @@ audio_close(struct audio_softc *sc, audio_file_t *file)
|
|||
/* 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 @@ audio_read(struct audio_softc *sc, struct uio *uio, int ioflag,
|
|||
TRACET(2, track, "resid=%zd", uio->uio_resid);
|
||||
|
||||
KASSERT(!mutex_owned(sc->sc_lock));
|
||||
KASSERT(file->lock);
|
||||
|
||||
/* I think it's better than EINVAL. */
|
||||
if (track->mmapped)
|
||||
|
|
@ -2367,7 +2257,6 @@ audio_read(struct audio_softc *sc, struct uio *uio, int ioflag,
|
|||
|
||||
audio_track_lock_enter(track);
|
||||
audio_track_record(track);
|
||||
audio_track_lock_exit(track);
|
||||
|
||||
/* uiomove from usrbuf as much as possible. */
|
||||
bytes = uimin(usrbuf->used, uio->uio_resid);
|
||||
|
|
@ -2377,6 +2266,7 @@ audio_read(struct audio_softc *sc, struct uio *uio, int ioflag,
|
|||
error = uiomove((uint8_t *)usrbuf->mem + head, len,
|
||||
uio);
|
||||
if (error) {
|
||||
audio_track_lock_exit(track);
|
||||
device_printf(sc->sc_dev,
|
||||
"uiomove(len=%d) failed with %d\n",
|
||||
len, error);
|
||||
|
|
@ -2389,6 +2279,8 @@ audio_read(struct audio_softc *sc, struct uio *uio, int ioflag,
|
|||
usrbuf->head, usrbuf->used, usrbuf->capacity);
|
||||
bytes -= len;
|
||||
}
|
||||
|
||||
audio_track_lock_exit(track);
|
||||
}
|
||||
|
||||
abort:
|
||||
|
|
@ -2425,7 +2317,6 @@ audio_write(struct audio_softc *sc, struct uio *uio, int ioflag,
|
|||
uio->uio_resid, (int)curproc->p_pid, (int)curlwp->l_lid, ioflag);
|
||||
|
||||
KASSERT(!mutex_owned(sc->sc_lock));
|
||||
KASSERT(file->lock);
|
||||
|
||||
/* I think it's better than EINVAL. */
|
||||
if (track->mmapped)
|
||||
|
|
@ -2492,6 +2383,8 @@ audio_write(struct audio_softc *sc, struct uio *uio, int ioflag,
|
|||
}
|
||||
mutex_exit(sc->sc_lock);
|
||||
|
||||
audio_track_lock_enter(track);
|
||||
|
||||
/* uiomove to usrbuf as much as possible. */
|
||||
bytes = uimin(track->usrbuf_usedhigh - usrbuf->used,
|
||||
uio->uio_resid);
|
||||
|
|
@ -2501,6 +2394,7 @@ audio_write(struct audio_softc *sc, struct uio *uio, int ioflag,
|
|||
error = uiomove((uint8_t *)usrbuf->mem + tail, len,
|
||||
uio);
|
||||
if (error) {
|
||||
audio_track_lock_exit(track);
|
||||
device_printf(sc->sc_dev,
|
||||
"uiomove(len=%d) failed with %d\n",
|
||||
len, error);
|
||||
|
|
@ -2515,11 +2409,11 @@ audio_write(struct audio_softc *sc, struct uio *uio, int ioflag,
|
|||
}
|
||||
|
||||
/* Convert them as much as possible. */
|
||||
audio_track_lock_enter(track);
|
||||
while (usrbuf->used >= track->usrbuf_blksize &&
|
||||
outbuf->used < outbuf->capacity) {
|
||||
audio_track_play(track);
|
||||
}
|
||||
|
||||
audio_track_lock_exit(track);
|
||||
}
|
||||
|
||||
|
|
@ -2544,7 +2438,6 @@ audio_ioctl(dev_t dev, struct audio_softc *sc, u_long cmd, void *addr, int flag,
|
|||
int error;
|
||||
|
||||
KASSERT(!mutex_owned(sc->sc_lock));
|
||||
KASSERT(file->lock);
|
||||
|
||||
#if defined(AUDIO_DEBUG)
|
||||
const char *ioctlnames[] = {
|
||||
|
|
@ -2849,7 +2742,6 @@ audio_poll(struct audio_softc *sc, int events, struct lwp *l,
|
|||
bool out_is_valid;
|
||||
|
||||
KASSERT(!mutex_owned(sc->sc_lock));
|
||||
KASSERT(file->lock);
|
||||
|
||||
#if defined(AUDIO_DEBUG)
|
||||
#define POLLEV_BITMAP "\177\020" \
|
||||
|
|
@ -3004,7 +2896,6 @@ audio_kqfilter(struct audio_softc *sc, audio_file_t *file, struct knote *kn)
|
|||
struct klist *klist;
|
||||
|
||||
KASSERT(!mutex_owned(sc->sc_lock));
|
||||
KASSERT(file->lock);
|
||||
|
||||
TRACEF(3, file, "kn=%p kn_filter=%x", kn, (int)kn->kn_filter);
|
||||
|
||||
|
|
@ -3042,7 +2933,6 @@ audio_mmap(struct audio_softc *sc, off_t *offp, size_t len, int prot,
|
|||
int error;
|
||||
|
||||
KASSERT(!mutex_owned(sc->sc_lock));
|
||||
KASSERT(file->lock);
|
||||
|
||||
TRACEF(2, file, "off=%lld, prot=%d", (long long)(*offp), prot);
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
/* $NetBSD: audiodef.h,v 1.2 2019/05/08 13:40:17 isaki Exp $ */
|
||||
/* $NetBSD: audiodef.h,v 1.3 2019/05/23 12:20:27 isaki Exp $ */
|
||||
|
||||
/*
|
||||
* Copyright (C) 2017 Tetsuya Isaki. All rights reserved.
|
||||
|
|
@ -182,13 +182,6 @@ struct audio_file {
|
|||
/* process who wants audio SIGIO. */
|
||||
pid_t async_audio;
|
||||
|
||||
/*
|
||||
* Non-zero if some thread context is using this file structure
|
||||
* (including ptrack and rtrack) now.
|
||||
* Must be protected by sc_lock.
|
||||
*/
|
||||
volatile int lock;
|
||||
|
||||
SLIST_ENTRY(audio_file) entry;
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue