Skip to contentSkip to conversation

Patch "ALSA: pcm: Fix rwsem deadlock for non-atomic PCM stream" has been added to the 4.4-stable tree

PATCH [email protected] 1 participant 1 message Feb 24, 2016

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

ALSA: pcm: Fix rwsem deadlock for non-atomic PCM stream

to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
alsa-pcm-fix-rwsem-deadlock-for-non-atomic-pcm-stream.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 67ec1072b053c15564e6090ab30127895dc77a89 Mon Sep 17 00:00:00 2001

From: Takashi Iwai <[email protected]>
Date: Wed, 17 Feb 2016 14:30:26 +0100
Subject: ALSA: pcm: Fix rwsem deadlock for non-atomic PCM stream

From: Takashi Iwai <[email protected]>

commit 67ec1072b053c15564e6090ab30127895dc77a89 upstream.

A non-atomic PCM stream may take snd_pcm_link_rwsem rw semaphore twice in the same code path, e.g. one in snd_pcm_action_nonatomic() and another in snd_pcm_stream_lock(). Usually this is OK, but when a write lock is issued between these two read locks, the problem
happens: the write lock is blocked due to the first reade lock, and the second read lock is also blocked by the write lock. This eventually deadlocks.

The reason is the way rwsem manages waiters; it's queued like FIFO, so even if the writer itself doesn't take the lock yet, it blocks all the waiters (including reads) queued after it.

As a workaround, in this patch, we replace the standard down_write() with an spinning loop. This is far from optimal, but it's good enough, as the spinning time is supposed to be relatively short for normal PCM operations, and the code paths requiring the write lock aren't called so often.

---
 sound/core/pcm_native.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
sound/core/pcm_native.c +14 -2
@@ -74,6 +74,18 @@ static int snd_pcm_open(struct file *fil
 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
+ * spin until it gets the lock.
+ */
+static inline void down_write_nonblock(struct rw_semaphore *lock)
+{
+	while (!down_write_trylock(lock))
+		cond_resched();
+}
+
 /**
  * snd_pcm_stream_lock - Lock the PCM stream
  * @substream: PCM substream
@@ -1813,7 +1825,7 @@ static int snd_pcm_link(struct snd_pcm_s
 		res = -ENOMEM;
 		goto _nolock;
 	}
-	down_write(&snd_pcm_link_rwsem);
+	down_write_nonblock(&snd_pcm_link_rwsem);
 	write_lock_irq(&snd_pcm_link_rwlock);
 	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN ||
 	    substream->runtime->status->state != substream1->runtime->status->state ||
@@ -1860,7 +1872,7 @@ static int snd_pcm_unlink(struct snd_pcm
 	struct snd_pcm_substream *s;
 	int res = 0;
 
-	down_write(&snd_pcm_link_rwsem);
+	down_write_nonblock(&snd_pcm_link_rwsem);
 	write_lock_irq(&snd_pcm_link_rwlock);
 	if (!snd_pcm_stream_linked(substream)) {
 		res = -EALREADY;

Patches currently in stable-queue which might be from [email protected] are

queue-4.4/alsa-seq-fix-leak-of-pool-buffer-at-concurrent-writes.patch queue-4.4/alsa-pcm-fix-rwsem-deadlock-for-non-atomic-pcm-stream.patch queue-4.4/alsa-hda-cancel-probe-work-instead-of-flush-at-remove.patch queue-4.4/alsa-seq-fix-double-port-list-deletion.patch