diff --git a/include/sound/pcm.h b/include/sound/pcm.h index eae6d2b82d7a..ca20f80f8976 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -30,6 +30,7 @@ #include #include #include +#include #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 bca0bdf3e33c..4f45b3000347 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 c515612969a4..0b4b5dfaec18 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 63640d3af9db..0bc4aa0ac9cf 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; }