Commit b5d3b760 authored by Chanho Min's avatar Chanho Min Committed by Kleber Sacilotto de Souza

ALSA: pcm: Fix starvation on down_write_nonblock()

BugLink: https://bugs.launchpad.net/bugs/1811077

commit b888a5f7 upstream.

Commit 67ec1072 ("ALSA: pcm: Fix rwsem deadlock for non-atomic PCM
stream") fixes deadlock for non-atomic PCM stream. But, This patch
causes antother stuck.
If writer is RT thread and reader is a normal thread, the reader
thread will be difficult to get scheduled. It may not give chance to
release readlocks and writer gets stuck for a long time if they are
pinned to single cpu.

The deadlock described in the previous commit is because the linux
rwsem queues like a FIFO. So, we might need non-FIFO writelock, not
non-block one.

My suggestion is that the writer gives reader a chance to be scheduled
by using the minimum msleep() instaed of spinning without blocking by
writer. Also, The *_nonblock may be changed to *_nonfifo appropriately
to this concept.
In terms of performance, when trylock is failed, this minimum periodic
msleep will have the same performance as the tick-based
schedule()/wake_up_q().

[ Although this has a fairly high performance penalty, the relevant
  code path became already rare due to the previous commit ("ALSA:
  pcm: Call snd_pcm_unlink() conditionally at closing").  That is, now
  this unconditional msleep appears only when using linked streams,
  and this must be a rare case.  So we accept this as a quick
  workaround until finding a more suitable one -- tiwai ]

Fixes: 67ec1072 ("ALSA: pcm: Fix rwsem deadlock for non-atomic PCM stream")
Suggested-by: default avatarWonmin Jung <wonmin.jung@lge.com>
Signed-off-by: default avatarChanho Min <chanho.min@lge.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarJuerg Haefliger <juergh@canonical.com>
Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
parent 22553f07
...@@ -35,6 +35,7 @@ ...@@ -35,6 +35,7 @@
#include <sound/timer.h> #include <sound/timer.h>
#include <sound/minors.h> #include <sound/minors.h>
#include <linux/uio.h> #include <linux/uio.h>
#include <linux/delay.h>
/* /*
* Compatibility * Compatibility
...@@ -78,12 +79,12 @@ static DECLARE_RWSEM(snd_pcm_link_rwsem); ...@@ -78,12 +79,12 @@ static DECLARE_RWSEM(snd_pcm_link_rwsem);
* and this may lead to a deadlock when the code path takes read sem * 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 * twice (e.g. one in snd_pcm_action_nonatomic() and another in
* snd_pcm_stream_lock()). As a (suboptimal) workaround, let writer to * snd_pcm_stream_lock()). As a (suboptimal) workaround, let writer to
* spin until it gets the lock. * sleep until all the readers are completed without blocking by writer.
*/ */
static inline void down_write_nonblock(struct rw_semaphore *lock) static inline void down_write_nonfifo(struct rw_semaphore *lock)
{ {
while (!down_write_trylock(lock)) while (!down_write_trylock(lock))
cond_resched(); msleep(1);
} }
/** /**
...@@ -1825,7 +1826,7 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd) ...@@ -1825,7 +1826,7 @@ static int snd_pcm_link(struct snd_pcm_substream *substream, int fd)
res = -ENOMEM; res = -ENOMEM;
goto _nolock; goto _nolock;
} }
down_write_nonblock(&snd_pcm_link_rwsem); down_write_nonfifo(&snd_pcm_link_rwsem);
write_lock_irq(&snd_pcm_link_rwlock); write_lock_irq(&snd_pcm_link_rwlock);
if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN || if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN ||
substream->runtime->status->state != substream1->runtime->status->state || substream->runtime->status->state != substream1->runtime->status->state ||
...@@ -1872,7 +1873,7 @@ static int snd_pcm_unlink(struct snd_pcm_substream *substream) ...@@ -1872,7 +1873,7 @@ static int snd_pcm_unlink(struct snd_pcm_substream *substream)
struct snd_pcm_substream *s; struct snd_pcm_substream *s;
int res = 0; int res = 0;
down_write_nonblock(&snd_pcm_link_rwsem); down_write_nonfifo(&snd_pcm_link_rwsem);
write_lock_irq(&snd_pcm_link_rwlock); write_lock_irq(&snd_pcm_link_rwlock);
if (!snd_pcm_stream_linked(substream)) { if (!snd_pcm_stream_linked(substream)) {
res = -EALREADY; res = -EALREADY;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment