From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: lock-up when loading desktop Date: Mon, 13 Oct 2014 23:14:14 +0200 Message-ID: References: <543B2438.1050002@internode.on.net> <543BB942.5060602@internode.on.net> <543BCDE9.7010308@internode.on.net> <543C153A.4060607@internode.on.net> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 678432604E2 for ; Mon, 13 Oct 2014 23:14:15 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Arthur Marsh Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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 > > 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.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 > > 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.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);