Merge branch 'topic/pcm-lock-refactor' into for-next
Pull PCM lock refactoring.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index eae6d2b..ca20f80 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -30,6 +30,7 @@
#include <linux/mm.h>
#include <linux/bitops.h>
#include <linux/pm_qos.h>
+#include <linux/refcount.h>
#define snd_pcm_substream_chip(substream) ((substream)->private_data)
#define snd_pcm_chip(pcm) ((pcm)->private_data)
@@ -439,7 +440,7 @@ struct snd_pcm_group { /* keep linked substreams */
spinlock_t lock;
struct mutex mutex;
struct list_head substreams;
- int count;
+ refcount_t refs;
};
struct pid;
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index bca0bdf..4f45b30 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -733,9 +733,7 @@ int snd_pcm_new_stream(struct snd_pcm *pcm, int stream, int substream_count)
}
}
substream->group = &substream->self_group;
- spin_lock_init(&substream->self_group.lock);
- mutex_init(&substream->self_group.mutex);
- INIT_LIST_HEAD(&substream->self_group.substreams);
+ snd_pcm_group_init(&substream->self_group);
list_add_tail(&substream->link_list, &substream->self_group.substreams);
atomic_set(&substream->mmap_count, 0);
prev = substream;
diff --git a/sound/core/pcm_local.h b/sound/core/pcm_local.h
index c515612..0b4b5df 100644
--- a/sound/core/pcm_local.h
+++ b/sound/core/pcm_local.h
@@ -66,5 +66,6 @@ static inline void snd_pcm_timer_done(struct snd_pcm_substream *substream) {}
#endif
void __snd_pcm_xrun(struct snd_pcm_substream *substream);
+void snd_pcm_group_init(struct snd_pcm_group *group);
#endif /* __SOUND_CORE_PCM_LOCAL_H */
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 63640d3..0bc4aa0 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -85,71 +85,30 @@ static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream);
*
*/
-static DEFINE_RWLOCK(snd_pcm_link_rwlock);
static DECLARE_RWSEM(snd_pcm_link_rwsem);
-/* Writer in rwsem may block readers even during its waiting in queue,
- * and this may lead to a deadlock when the code path takes read sem
- * twice (e.g. one in snd_pcm_action_nonatomic() and another in
- * snd_pcm_stream_lock()). As a (suboptimal) workaround, let writer to
- * sleep until all the readers are completed without blocking by writer.
- */
-static inline void down_write_nonfifo(struct rw_semaphore *lock)
+void snd_pcm_group_init(struct snd_pcm_group *group)
{
- while (!down_write_trylock(lock))
- msleep(1);
+ spin_lock_init(&group->lock);
+ mutex_init(&group->mutex);
+ INIT_LIST_HEAD(&group->substreams);
+ refcount_set(&group->refs, 0);
}
-#define PCM_LOCK_DEFAULT 0
-#define PCM_LOCK_IRQ 1
-#define PCM_LOCK_IRQSAVE 2
-
-static unsigned long __snd_pcm_stream_lock_mode(struct snd_pcm_substream *substream,
- unsigned int mode)
-{
- unsigned long flags = 0;
- if (substream->pcm->nonatomic) {
- down_read_nested(&snd_pcm_link_rwsem, SINGLE_DEPTH_NESTING);
- mutex_lock(&substream->self_group.mutex);
- } else {
- switch (mode) {
- case PCM_LOCK_DEFAULT:
- read_lock(&snd_pcm_link_rwlock);
- break;
- case PCM_LOCK_IRQ:
- read_lock_irq(&snd_pcm_link_rwlock);
- break;
- case PCM_LOCK_IRQSAVE:
- read_lock_irqsave(&snd_pcm_link_rwlock, flags);
- break;
- }
- spin_lock(&substream->self_group.lock);
- }
- return flags;
+/* define group lock helpers */
+#define DEFINE_PCM_GROUP_LOCK(action, mutex_action) \
+static void snd_pcm_group_ ## action(struct snd_pcm_group *group, bool nonatomic) \
+{ \
+ if (nonatomic) \
+ mutex_ ## mutex_action(&group->mutex); \
+ else \
+ spin_ ## action(&group->lock); \
}
-static void __snd_pcm_stream_unlock_mode(struct snd_pcm_substream *substream,
- unsigned int mode, unsigned long flags)
-{
- if (substream->pcm->nonatomic) {
- mutex_unlock(&substream->self_group.mutex);
- up_read(&snd_pcm_link_rwsem);
- } else {
- spin_unlock(&substream->self_group.lock);
-
- switch (mode) {
- case PCM_LOCK_DEFAULT:
- read_unlock(&snd_pcm_link_rwlock);
- break;
- case PCM_LOCK_IRQ:
- read_unlock_irq(&snd_pcm_link_rwlock);
- break;
- case PCM_LOCK_IRQSAVE:
- read_unlock_irqrestore(&snd_pcm_link_rwlock, flags);
- break;
- }
- }
-}
+DEFINE_PCM_GROUP_LOCK(lock, lock);
+DEFINE_PCM_GROUP_LOCK(unlock, unlock);
+DEFINE_PCM_GROUP_LOCK(lock_irq, lock);
+DEFINE_PCM_GROUP_LOCK(unlock_irq, unlock);
/**
* snd_pcm_stream_lock - Lock the PCM stream
@@ -161,7 +120,7 @@ static void __snd_pcm_stream_unlock_mode(struct snd_pcm_substream *substream,
*/
void snd_pcm_stream_lock(struct snd_pcm_substream *substream)
{
- __snd_pcm_stream_lock_mode(substream, PCM_LOCK_DEFAULT);
+ snd_pcm_group_lock(&substream->self_group, substream->pcm->nonatomic);
}
EXPORT_SYMBOL_GPL(snd_pcm_stream_lock);
@@ -173,7 +132,7 @@ EXPORT_SYMBOL_GPL(snd_pcm_stream_lock);
*/
void snd_pcm_stream_unlock(struct snd_pcm_substream *substream)
{
- __snd_pcm_stream_unlock_mode(substream, PCM_LOCK_DEFAULT, 0);
+ snd_pcm_group_unlock(&substream->self_group, substream->pcm->nonatomic);
}
EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock);
@@ -187,7 +146,8 @@ EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock);
*/
void snd_pcm_stream_lock_irq(struct snd_pcm_substream *substream)
{
- __snd_pcm_stream_lock_mode(substream, PCM_LOCK_IRQ);
+ snd_pcm_group_lock_irq(&substream->self_group,
+ substream->pcm->nonatomic);
}
EXPORT_SYMBOL_GPL(snd_pcm_stream_lock_irq);
@@ -199,13 +159,19 @@ EXPORT_SYMBOL_GPL(snd_pcm_stream_lock_irq);
*/
void snd_pcm_stream_unlock_irq(struct snd_pcm_substream *substream)
{
- __snd_pcm_stream_unlock_mode(substream, PCM_LOCK_IRQ, 0);
+ snd_pcm_group_unlock_irq(&substream->self_group,
+ substream->pcm->nonatomic);
}
EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock_irq);
unsigned long _snd_pcm_stream_lock_irqsave(struct snd_pcm_substream *substream)
{
- return __snd_pcm_stream_lock_mode(substream, PCM_LOCK_IRQSAVE);
+ unsigned long flags = 0;
+ if (substream->pcm->nonatomic)
+ mutex_lock(&substream->self_group.mutex);
+ else
+ spin_lock_irqsave(&substream->self_group.lock, flags);
+ return flags;
}
EXPORT_SYMBOL_GPL(_snd_pcm_stream_lock_irqsave);
@@ -219,7 +185,10 @@ EXPORT_SYMBOL_GPL(_snd_pcm_stream_lock_irqsave);
void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream,
unsigned long flags)
{
- __snd_pcm_stream_unlock_mode(substream, PCM_LOCK_IRQSAVE, flags);
+ if (substream->pcm->nonatomic)
+ mutex_unlock(&substream->self_group.mutex);
+ else
+ spin_unlock_irqrestore(&substream->self_group.lock, flags);
}
EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock_irqrestore);
@@ -1124,6 +1093,68 @@ static int snd_pcm_action_single(const struct action_ops *ops,
return res;
}
+static void snd_pcm_group_assign(struct snd_pcm_substream *substream,
+ struct snd_pcm_group *new_group)
+{
+ substream->group = new_group;
+ list_move(&substream->link_list, &new_group->substreams);
+}
+
+/*
+ * Unref and unlock the group, but keep the stream lock;
+ * when the group becomes empty and no longer referred, destroy itself
+ */
+static void snd_pcm_group_unref(struct snd_pcm_group *group,
+ struct snd_pcm_substream *substream)
+{
+ bool do_free;
+
+ if (!group)
+ return;
+ do_free = refcount_dec_and_test(&group->refs) &&
+ list_empty(&group->substreams);
+ snd_pcm_group_unlock(group, substream->pcm->nonatomic);
+ if (do_free)
+ kfree(group);
+}
+
+/*
+ * Lock the group inside a stream lock and reference it;
+ * return the locked group object, or NULL if not linked
+ */
+static struct snd_pcm_group *
+snd_pcm_stream_group_ref(struct snd_pcm_substream *substream)
+{
+ bool nonatomic = substream->pcm->nonatomic;
+ struct snd_pcm_group *group;
+ bool trylock;
+
+ for (;;) {
+ if (!snd_pcm_stream_linked(substream))
+ return NULL;
+ group = substream->group;
+ /* block freeing the group object */
+ refcount_inc(&group->refs);
+
+ trylock = nonatomic ? mutex_trylock(&group->mutex) :
+ spin_trylock(&group->lock);
+ if (trylock)
+ break; /* OK */
+
+ /* re-lock for avoiding ABBA deadlock */
+ snd_pcm_stream_unlock(substream);
+ snd_pcm_group_lock(group, nonatomic);
+ snd_pcm_stream_lock(substream);
+
+ /* check the group again; the above opens a small race window */
+ if (substream->group == group)
+ break; /* OK */
+ /* group changed, try again */
+ snd_pcm_group_unref(group, substream);
+ }
+ return group;
+}
+
/*
* Note: call with stream lock
*/
@@ -1131,28 +1162,15 @@ static int snd_pcm_action(const struct action_ops *ops,
struct snd_pcm_substream *substream,
int state)
{
+ struct snd_pcm_group *group;
int res;
- if (!snd_pcm_stream_linked(substream))
- return snd_pcm_action_single(ops, substream, state);
-
- if (substream->pcm->nonatomic) {
- if (!mutex_trylock(&substream->group->mutex)) {
- mutex_unlock(&substream->self_group.mutex);
- mutex_lock(&substream->group->mutex);
- mutex_lock(&substream->self_group.mutex);
- }
+ group = snd_pcm_stream_group_ref(substream);
+ if (group)
res = snd_pcm_action_group(ops, substream, state, 1);
- mutex_unlock(&substream->group->mutex);
- } else {
- if (!spin_trylock(&substream->group->lock)) {
- spin_unlock(&substream->self_group.lock);
- spin_lock(&substream->group->lock);
- spin_lock(&substream->self_group.lock);
- }
- res = snd_pcm_action_group(ops, substream, state, 1);
- spin_unlock(&substream->group->lock);
- }
+ else
+ res = snd_pcm_action_single(ops, substream, state);
+ snd_pcm_group_unref(group, substream);
return res;
}
@@ -1179,6 +1197,7 @@ static int snd_pcm_action_nonatomic(const struct action_ops *ops,
{
int res;
+ /* Guarantee the group members won't change during non-atomic action */
down_read(&snd_pcm_link_rwsem);
if (snd_pcm_stream_linked(substream))
res = snd_pcm_action_group(ops, substream, state, 0);
@@ -1802,6 +1821,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
struct snd_card *card;
struct snd_pcm_runtime *runtime;
struct snd_pcm_substream *s;
+ struct snd_pcm_group *group;
wait_queue_entry_t wait;
int result = 0;
int nonblock = 0;
@@ -1818,7 +1838,6 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
} else if (substream->f_flags & O_NONBLOCK)
nonblock = 1;
- down_read(&snd_pcm_link_rwsem);
snd_pcm_stream_lock_irq(substream);
/* resume pause */
if (runtime->status->state == SNDRV_PCM_STATE_PAUSED)
@@ -1843,6 +1862,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
}
/* find a substream to drain */
to_check = NULL;
+ group = snd_pcm_stream_group_ref(substream);
snd_pcm_group_for_each_entry(s, substream) {
if (s->stream != SNDRV_PCM_STREAM_PLAYBACK)
continue;
@@ -1852,12 +1872,12 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
break;
}
}
+ snd_pcm_group_unref(group, substream);
if (!to_check)
break; /* all drained */
init_waitqueue_entry(&wait, current);
add_wait_queue(&to_check->sleep, &wait);
snd_pcm_stream_unlock_irq(substream);
- up_read(&snd_pcm_link_rwsem);
if (runtime->no_period_wakeup)
tout = MAX_SCHEDULE_TIMEOUT;
else {
@@ -1869,9 +1889,17 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
tout = msecs_to_jiffies(tout * 1000);
}
tout = schedule_timeout_interruptible(tout);
- down_read(&snd_pcm_link_rwsem);
+
snd_pcm_stream_lock_irq(substream);
- remove_wait_queue(&to_check->sleep, &wait);
+ group = snd_pcm_stream_group_ref(substream);
+ snd_pcm_group_for_each_entry(s, substream) {
+ if (s->runtime == to_check) {
+ remove_wait_queue(&to_check->sleep, &wait);
+ break;
+ }
+ }
+ snd_pcm_group_unref(group, substream);
+
if (card->shutdown) {
result = -ENODEV;
break;
@@ -1891,7 +1919,6 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
unlock:
snd_pcm_stream_unlock_irq(substream);
- up_read(&snd_pcm_link_rwsem);
return result;
}
@@ -1930,13 +1957,19 @@ static int snd_pcm_drop(struct snd_pcm_substream *substream)
static bool is_pcm_file(struct file *file)
{
struct inode *inode = file_inode(file);
+ struct snd_pcm *pcm;
unsigned int minor;
if (!S_ISCHR(inode->i_mode) || imajor(inode) != snd_major)
return false;
minor = iminor(inode);
- return snd_lookup_minor_data(minor, SNDRV_DEVICE_TYPE_PCM_PLAYBACK) ||
- snd_lookup_minor_data(minor, SNDRV_DEVICE_TYPE_PCM_CAPTURE);
+ pcm = snd_lookup_minor_data(minor, SNDRV_DEVICE_TYPE_PCM_PLAYBACK);
+ if (!pcm)
+ pcm = snd_lookup_minor_data(minor, SNDRV_DEVICE_TYPE_PCM_CAPTURE);
+ if (!pcm)
+ return false;
+ snd_card_unref(pcm->card);
+ return true;
}
/*
@@ -1947,7 +1980,8 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
int res = 0;
struct snd_pcm_file *pcm_file;
struct snd_pcm_substream *substream1;
- struct snd_pcm_group *group;
+ struct snd_pcm_group *group, *target_group;
+ bool nonatomic = substream->pcm->nonatomic;
struct fd f = fdget(fd);
if (!f.file)
@@ -1958,13 +1992,14 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
}
pcm_file = f.file->private_data;
substream1 = pcm_file->substream;
- group = kmalloc(sizeof(*group), GFP_KERNEL);
+ group = kzalloc(sizeof(*group), GFP_KERNEL);
if (!group) {
res = -ENOMEM;
goto _nolock;
}
- down_write_nonfifo(&snd_pcm_link_rwsem);
- write_lock_irq(&snd_pcm_link_rwlock);
+ snd_pcm_group_init(group);
+
+ down_write(&snd_pcm_link_rwsem);
if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN ||
substream->runtime->status->state != substream1->runtime->status->state ||
substream->pcm->nonatomic != substream1->pcm->nonatomic) {
@@ -1975,23 +2010,23 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
res = -EALREADY;
goto _end;
}
+
+ snd_pcm_stream_lock_irq(substream);
if (!snd_pcm_stream_linked(substream)) {
- substream->group = group;
- group = NULL;
- spin_lock_init(&substream->group->lock);
- mutex_init(&substream->group->mutex);
- INIT_LIST_HEAD(&substream->group->substreams);
- list_add_tail(&substream->link_list, &substream->group->substreams);
- substream->group->count = 1;
+ snd_pcm_group_assign(substream, group);
+ group = NULL; /* assigned, don't free this one below */
}
- list_add_tail(&substream1->link_list, &substream->group->substreams);
- substream->group->count++;
- substream1->group = substream->group;
+ target_group = substream->group;
+ snd_pcm_stream_unlock_irq(substream);
+
+ snd_pcm_group_lock_irq(target_group, nonatomic);
+ snd_pcm_stream_lock(substream1);
+ snd_pcm_group_assign(substream1, target_group);
+ snd_pcm_stream_unlock(substream1);
+ snd_pcm_group_unlock_irq(target_group, nonatomic);
_end:
- write_unlock_irq(&snd_pcm_link_rwlock);
up_write(&snd_pcm_link_rwsem);
_nolock:
- snd_card_unref(substream1->pcm->card);
kfree(group);
_badf:
fdput(f);
@@ -2000,34 +2035,43 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
static void relink_to_local(struct snd_pcm_substream *substream)
{
- substream->group = &substream->self_group;
- INIT_LIST_HEAD(&substream->self_group.substreams);
- list_add_tail(&substream->link_list, &substream->self_group.substreams);
+ snd_pcm_stream_lock(substream);
+ snd_pcm_group_assign(substream, &substream->self_group);
+ snd_pcm_stream_unlock(substream);
}
static int snd_pcm_unlink(struct snd_pcm_substream *substream)
{
- struct snd_pcm_substream *s;
+ struct snd_pcm_group *group;
+ bool nonatomic = substream->pcm->nonatomic;
+ bool do_free = false;
int res = 0;
- down_write_nonfifo(&snd_pcm_link_rwsem);
- write_lock_irq(&snd_pcm_link_rwlock);
+ down_write(&snd_pcm_link_rwsem);
+
if (!snd_pcm_stream_linked(substream)) {
res = -EALREADY;
goto _end;
}
- list_del(&substream->link_list);
- substream->group->count--;
- if (substream->group->count == 1) { /* detach the last stream, too */
- snd_pcm_group_for_each_entry(s, substream) {
- relink_to_local(s);
- break;
- }
- kfree(substream->group);
- }
+
+ group = substream->group;
+ snd_pcm_group_lock_irq(group, nonatomic);
+
relink_to_local(substream);
+
+ /* detach the last stream, too */
+ if (list_is_singular(&group->substreams)) {
+ relink_to_local(list_first_entry(&group->substreams,
+ struct snd_pcm_substream,
+ link_list));
+ do_free = !refcount_read(&group->refs);
+ }
+
+ snd_pcm_group_unlock_irq(group, nonatomic);
+ if (do_free)
+ kfree(group);
+
_end:
- write_unlock_irq(&snd_pcm_link_rwlock);
up_write(&snd_pcm_link_rwsem);
return res;
}