At Tue, 14 Oct 2014 04:38:58 +1030, Arthur Marsh wrote:
Takashi Iwai wrote, on 14/10/14 00:55:
At Mon, 13 Oct 2014 23:34:41 +1030, Arthur Marsh wrote:
Takashi Iwai wrote, on 13/10/14 23:10:
OK, then could you check whether reverting only the last one (of two) does *NOT* fix the issue? I think it shouldn't, but let us confirm.
Thanks for the feedback. Sorry, I'm not good on git commands. Having applied the two reversions previously, how do I get my git tree back to the same state as Linus' git head (ie undo the reversions)?
Suppose your tree is now with two reverts, just run "git reset --hard HEAD~". Then it goes back in a single commit.
I could then apply the second reversion only and test that.
The alsa-info.sh shows the lockdep messages. I assume that it's seen even before 3.17+?
I didn't see ALSA-related lockdep messages in 3.17.0 or earlier, and have had the lockdep validator option enabled on kernels for several months now.
OK, so it's a new message. But this appears after you reverted two commits, right? Then the lock code base in sound core should be identical with 3.17. Strange. Do you see anything when you run
git diff v3.17.. sound/core/pcm_native.con the tree with two reverts?
Last but not least, please check whether nonatomic flag is set wrongly by some reason like the patch below. (Check the kernel message after boot or try to playback some PCM).
Takashi
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;
I can apply this patch, but I'm not entirely sure how to check for the state of the nonatomic flag.
If it were wrongly set, the PCM open would show the kernel warning and abort immediately, so you'll notice soon.
One more test would be to check the direct merge to 3.17 as I've tested.
Go to git tree, check out some branch % git checkout -b sound-test
Reset to vanilla 3.17 % git reset --hard v3.17
Merge only the sound changes onto it % git merge fd1a2a90d08b0052fa52bd36cebd0592c9e537c2
Give any merge messages as you like.
Compile and retest whether the same problem happens with this one.
thanks!
Takashi
OK
# 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@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@example.comAfter doing this, you may fix the identity used for this commit with:
git commit --amend --reset-author2 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@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@example.comAfter doing this, you may fix the identity used for this commit with:
git commit --amend --reset-author3 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? Could you test without this (but with the patch below)?
I then rebuilt that kernel, and installed it.
It booted and played its start-up sound of a MIDI file fine, but when I attempted to run alsa-info.sh it locked up (see first photo), and after rebooting again and trying to run:
aplay some-file.wav
had a similar lock-up (see second photo)
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"
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;