[alsa-devel] lock-up when loading desktop
Takashi Iwai
tiwai at suse.de
Mon Oct 13 22:57:59 CEST 2014
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.c
> >
> > on 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 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? 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;
More information about the Alsa-devel
mailing list