[alsa-devel] lock-up when loading desktop
Takashi Iwai
tiwai at suse.de
Mon Oct 13 23:14:14 CEST 2014
At Mon, 13 Oct 2014 22:57:59 +0200,
Takashi Iwai wrote:
>
> 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.
Also below is another thing I spotted now, but this is likely
irrelevant from the lockup, supposedly. In anyway, try this one
together with the previous patch, too.
Takashi
---
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 85fe1a216225..bfe1cf6b492f 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -818,7 +818,7 @@ static int snd_pcm_action_group(struct action_ops *ops,
/* unlock streams */
snd_pcm_group_for_each_entry(s1, substream) {
if (s1 != substream) {
- if (s->pcm->nonatomic)
+ if (s1->pcm->nonatomic)
mutex_unlock(&s1->self_group.mutex);
else
spin_unlock(&s1->self_group.lock);
More information about the Alsa-devel
mailing list