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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC075C433EF for ; Tue, 12 Oct 2021 06:35:08 +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 9830B60EFE for ; Tue, 12 Oct 2021 06:35:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9830B60EFE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=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 466771686; Tue, 12 Oct 2021 08:34:15 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 466771686 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1634020505; bh=vbSCWWVl8c4wUkbo0bp6yiI/GcTC8OBE54ZfUFp96FE=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=IXIagb4ZoqJOnnr5gGhhpA/BlYkbzX56W1AMfbIWuyRhQtC+Jg3WPC2IUqQ3tbZPo YbiiMaredMn4tXdz4O4lGGscIWAQuR2Gycx6zL+MEIQXtkuPQZhnS03tGOEfsI8FQA ZkRC/m9q/RbW3YMddsAQcj1UTt5m17/n+1cgS3uY= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id A33CAF801F7; Tue, 12 Oct 2021 08:34:14 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 1B059F800C0; Tue, 12 Oct 2021 08:34:13 +0200 (CEST) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 0D6D3F800C0 for ; Tue, 12 Oct 2021 08:34:08 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 0D6D3F800C0 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="Z4K6ZhbY"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="RpHxuzfi" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id A439421FEA; Tue, 12 Oct 2021 06:34:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1634020447; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=4Dj85ik7+vQL7IvqeOozxPFcclfR9RIkCKTYskVeaWs=; b=Z4K6ZhbY+sYm0PYnRnw4GVbAqQO6bKkNxh3lorkBN/gsO5aG9kgzQn2eM5nVArI0bBAb5m GKweYaUGfHNfmpN/PqYRuuR1xIhWbp5VA0loYPLrbijqekrKRb5UvhpE0mSvLSH0QL6unR WbqzzCDuHonAg0xR2nsWUNSXc0AeELY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1634020447; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=4Dj85ik7+vQL7IvqeOozxPFcclfR9RIkCKTYskVeaWs=; b=RpHxuzfidevCY5sl4e3CmdyYkhLJNpyHjN/tmUSz8FrptZYY1AbiiGEtGE4rMdqDI6Vyw4 wCfd7jg2xOdMXmCQ== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id 1FFE2A3B83; Tue, 12 Oct 2021 06:34:07 +0000 (UTC) Date: Tue, 12 Oct 2021 08:34:07 +0200 Message-ID: From: Takashi Iwai To: Pierre-Louis Bossart Subject: Re: [RFC PATCH v2 0/5] ASoC: soc-pcm: fix trigger race conditions with shared BE In-Reply-To: <8c1353f0-e897-e7b0-c7b9-5712b05ac91f@linux.intel.com> References: <20211004225441.233375-1-pierre-louis.bossart@linux.intel.com> <1efa1c31-7342-05f8-5f73-95e2462d4179@linux.intel.com> <3683cf39-632b-50df-c65d-63779c464850@nvidia.com> <11257d77-9975-3b00-94da-5dc1b5c95fc6@linux.intel.com> <80882fe6-ea30-43f6-8d83-8995dd28c748@linux.intel.com> <60c6a90b-290d-368c-ce61-4d86e70eaa78@linux.intel.com> <75894aba-ca1a-51d6-df7d-ad53fcd89f79@linux.intel.com> <29397354-dc5b-7837-c71b-df4bde707df2@linux.intel.com> <8c1353f0-e897-e7b0-c7b9-5712b05ac91f@linux.intel.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") Content-Type: text/plain; charset=US-ASCII Cc: alsa-devel@alsa-project.org, Kuninori Morimoto , Sameer Pujar , vkoul@kernel.org, broonie@kernel.org, Gyeongtaek Lee , Peter Ujfalusi 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: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Mon, 11 Oct 2021 22:06:51 +0200, Pierre-Louis Bossart wrote: > > > >>> Basically we need to protect two things: > >>> - The BE links > >>> - The concurrent accesses to BEs > >>> The former belongs to each FE that holds the links, and the FE stream > >>> lock would cover. The latter is rather a per-BE business. > >>> > >>> An oft-seen risk of multiple locks is deadlocking, but in this case, > >>> as long as we keep the lock order FE->BE, nothing wrong can happen. > >> > >> famous last words "nothing wrong can happen." :-) > >> > >> I already added a helper to do this FE lock, I can easily replace the > >> implementation to remove the spin_lock and use the FE PCM lock. > >> we might even add the lock in the definition of for_each_dpcm_be() to > >> avoid misses. > >> > >> Let me try this out today, thanks for the suggestions. > > > > well, it's not successful at all... > > > > When I replace the existing dpcm_lock with the FE PCM lock as you > > suggested, without any additional changes, speaker-test produces valid > > audio on the endpoints, but if I try a Ctrl-C or limit the number of > > loops with the '-l' option, I hear an endless loop on the same buffer > > and I have to power cycle my test device to stop the sound. > > > > See 2 patches attached, the first patch with the introduction of the > > helper works fine, the second with the use of the FE PCM lock doesn't. > > In hindsight I am glad I worked on minimal patches, one after the other, > > to identify problems. > > > > And when I add the BE lock, then nothing happens. Device stuck and no > > audio... > > > > There must be something we're missing related to the locking... > > And indeed there's a deadlock! > > snd_pcm_period_elapsed() takes the FE pcm stream lock, and will call > snd_pcm_trigger. Indeed, this would deadlock. > So if we also take the pcm stream lock in the BE > trigger, there's a conceptual deadlock: we painted ourselves in a corner > by using the same lock twice. > > Takashi, are you really sure we should protect these for_each_dpcm_be() > loops with the same pcm lock? The call within the FE lock is done only in dpcm_dai_trigger_fe_be(), and this should call dpcm_be_dai_trigger() as is. In other places, the calls are without FE lock, hence they can take the lock, e.g. create a variant dpcm_dai_trigger_fe_be_lock(). > it seems like asking for trouble to > revisit the ALSA core just to walking through a list of BEs? Would you > object to changing dpcm_lock as I suggested, but not interfering with > stream handling? That would work, too, it's just a pity to degrade the fine-grained locks that have been already taken into global locks... Takashi