[alsa-devel] lock-up when loading desktop

[Arthur Marsh]

Takashi Iwai wrote, on 14/10/14 07:27:

>> # git reset --hard HEAD~
>> HEAD is now at 77c688a Merge branch 'for-linus' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
>> am64:/usr/src/linux# git revert 7af142f752116e86adbe2073f2922d8265a77709
>> [master 927ab0d] Revert "ALSA: pcm: Uninline snd_pcm_stream_lock() and
>> _unlock()"
>>    Committer: root <root at am64.localdomain>
>> Your name and email address were configured automatically based
>> on your username and hostname. Please check that they are accurate.
>> You can suppress this message by setting them explicitly:
>>
>>       git config --global user.name "Your Name"
>>       git config --global user.email you at example.com
>>
>> After doing this, you may fix the identity used for this commit with:
>>
>>       git commit --amend --reset-author
>>
>>    2 files changed, 73 insertions(+), 72 deletions(-)
>> am64:/usr/src/linux# git revert 257f8cce5d40b811d229ed71602882baa0012808
>> [master e59721c] Revert "ALSA: pcm: Allow nonatomic trigger operations"
>>    Committer: root <root at am64.localdomain>
>> Your name and email address were configured automatically based
>> on your username and hostname. Please check that they are accurate.
>> You can suppress this message by setting them explicitly:
>>
>>       git config --global user.name "Your Name"
>>       git config --global user.email you at example.com
>>
>> After doing this, you may fix the identity used for this commit with:
>>
>>       git commit --amend --reset-author
>>
>>    3 files changed, 19 insertions(+), 116 deletions(-)
>> am64:/usr/src/linux# git diff v3.17.. sound/core/pcm_native.c
>> am64:/usr/src/linux# git reset --hard HEAD~
>> HEAD is now at 927ab0d Revert "ALSA: pcm: Uninline snd_pcm_stream_lock()
>> and _unlock()"
>> am64:/usr/src/linux# git reset --hard v3.17
>> Checking out files: 100% (6352/6352), done.
>> HEAD is now at bfe01a5 Linux 3.17
>> am64:/usr/src/linux# git checkout -b sound-test
>> Switched to a new branch 'sound-test'
>> am64:/usr/src/linux# git merge fd1a2a90d08b0052fa52bd36cebd0592c9e537c2
>> Updating bfe01a5..fd1a2a9
>> Fast-forward
>> [big list of files]
>>
>> /usr/src/linux# patch -p1 <../sound.patch
>> patching file sound/core/pcm_native.c
>
> Wait... which patch is this?

This one:

---
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 85fe1a216225..9c7cbd1b839e 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2275,6 +2275,9 @@ static int snd_pcm_open(struct file *file, struct 
snd_pcm *pcm, int stream)
  	int err;
  	wait_queue_t wait;

+	if (WARN_ON(pcm->nonatomic))
+		return -EINVAL;
+
  	if (pcm == NULL) {
  		err = -ENODEV;
  		goto __error1;



>  Could you test without this (but with
> the patch below)?

OK.

> I looked at the relevant code now, and this indeed seems like a
> deadlock.  But it's nothing new, the code is a decade old.  I wonder
> why it appears out of sudden.  Maybe the change of the spin lock path
> triggers.
>
> The patch below is the fix, just removing the superfluous spinlock.
>
>> I can supply a dmesg output of the machine with the test kernel before
>> attempting anything that might cause a lock-up if it's of use to you.
>>
>> PS, how do I get my git repositary out of "sound-test" branch and return
>> to Linus' git head?
>
> Just do "git checkout master"

Thanks!

>
>
> Takashi
>
> ---
> diff --git a/sound/pci/emu10k1/emu10k1_callback.c b/sound/pci/emu10k1/emu10k1_callback.c
> index 3f3ef38d9b6e..874cd76c7b7f 100644
> --- a/sound/pci/emu10k1/emu10k1_callback.c
> +++ b/sound/pci/emu10k1/emu10k1_callback.c
> @@ -85,6 +85,8 @@ snd_emu10k1_ops_setup(struct snd_emux *emux)
>    * get more voice for pcm
>    *
>    * terminate most inactive voice and give it as a pcm voice.
> + *
> + * voice_lock is already held.
>    */
>   int
>   snd_emu10k1_synth_get_voice(struct snd_emu10k1 *hw)
> @@ -92,12 +94,10 @@ snd_emu10k1_synth_get_voice(struct snd_emu10k1 *hw)
>   	struct snd_emux *emu;
>   	struct snd_emux_voice *vp;
>   	struct best_voice best[V_END];
> -	unsigned long flags;
>   	int i;
>
>   	emu = hw->synth;
>
> -	spin_lock_irqsave(&emu->voice_lock, flags);
>   	lookup_voices(emu, hw, best, 1); /* no OFF voices */
>   	for (i = 0; i < V_END; i++) {
>   		if (best[i].voice >= 0) {
> @@ -113,11 +113,9 @@ snd_emu10k1_synth_get_voice(struct snd_emu10k1 *hw)
>   			vp->emu->num_voices--;
>   			vp->ch = -1;
>   			vp->state = SNDRV_EMUX_ST_OFF;
> -			spin_unlock_irqrestore(&emu->voice_lock, flags);
>   			return ch;
>   		}
>   	}
> -	spin_unlock_irqrestore(&emu->voice_lock, flags);
>
>   	/* not found */
>   	return -ENOMEM;
>

I'll apply the above patch only first and report the results.

Thanks again,

Arthur.