From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 58C68C3A5A6 for ; Sun, 22 Sep 2019 20:26:48 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5134F20882 for ; Sun, 22 Sep 2019 20:26:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="fi+hxM8R" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5134F20882 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id EA8FF83B; Sun, 22 Sep 2019 22:25:54 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz EA8FF83B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1569184005; bh=YKphgWz4z3JkS6HQg0K0og7Q9daKCmVUX0pKuzmv6xE=; h=Date:From:To:In-Reply-To:References:Cc:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=fi+hxM8Rc7i0Gc0hsep4jvdteJE3Ioir6d1JfbadZTG8VR/6CDGvjAuLdxGs3zChR 046bJ7KF9KnTmqLW5siHrPIKLGIjUkynzmKnYQZUPWndwTytZERfvE49ebRu3TSBoC xVYe377meSlvVuzwQn6FCj8j89h6qM+fMH/zJT/M= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 63D60F80307; Sun, 22 Sep 2019 22:25:54 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 6A532F8045F; Sun, 22 Sep 2019 22:25:52 +0200 (CEST) Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id B0E62F802BD for ; Sun, 22 Sep 2019 22:25:49 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz B0E62F802BD X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 7322EAD63; Sun, 22 Sep 2019 20:25:48 +0000 (UTC) Date: Sun, 22 Sep 2019 22:25:48 +0200 Message-ID: From: Takashi Iwai To: Ben Russell In-Reply-To: <20190922032853.6123-1-thematrixeatsyou@gmail.com> References: <20190922032853.6123-1-thematrixeatsyou@gmail.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] [PATCH 0/3] Make pcm_ioplug check lock status before locking (fixes pcm_jack lockups) X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Sun, 22 Sep 2019 05:28:50 +0200, Ben Russell wrote: > > This is my first time contributing a patch to a mailing list. If I've made a mess, please let me know so I can learn how to avoid it in future. > > The purpose of this patchset is to fix a specific common lockup in the pcm_jack plugin (from alsa-plugins). I'm not sure if this is the right approach to take, but at the very least it should make the pcm_ioplug code a bit more resilient. > > The problem is this: When using the pcm_jack plugin, if a program attempts to play audio using the SND_PCM_FORMAT_FLOAT format, said program locks up. > > This should be enough to reproduce the bug: > > pcm.rawjack { > type jack > playback_ports { > 0 system:playback_1 > 1 system:playback_2 > } > capture_ports { > 0 system:capture_1 > 1 system:capture_2 > } > } > > pcm.!default { > type plug > slave.pcm "rawjack" > } > > What's happening is that several snd_pcm_ioplug_* functions assume that the pcm mutex is locked already. It then proceeds to unlock the mutex, call a function, and then relock the mutex. When the mutex isn't locked already, the initial unlock results in a silently ignored pthread error, and the lock results in the program eventually deadlocking as it doesn't expect the mutex to be held at that point. > > Patch 2 modifies pcm_ioplug to check if the mutex is held before doing the unlock-act-lock sequence, and if the mutex is not held then it skips the unlock and lock stages. This depends on Patch 1, which adds a snd_pcm_is_locked function to give the state of the mutex. > > Patch 3 is completely optional. It adds assertions which make sure that all uses of snd_pcm_lock/snd_pcm_unlock are correct. On one hand this will likely result in crashes in some of the less refined parts of the code. On the other hand, when that happens, you'll know which parts need a bit more love. I know it was useful for finding this issue in the first place. > > These patches fix the problems I am having, but if you have a more suitable approach to fixing this problem then please let me know. Thanks for the analysis and patches. It's indeed a serious problem we need to address. The current semantics of locking in alsa-lib code assumes that the lock/unlock never fails. So the "right-and-quick" fix would be just to take the patch 3 to assert() upon pthread_mutex_lock/unlock errors. Then we need to paper over the actual invalid locks. I do wonder, though, exactly which code path triggers the pthread mutex error? You should be able to catch it via gdb after the patch. thanks, Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel