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 1EC96C433EF for ; Fri, 8 Oct 2021 14:52:27 +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 5C43C61039 for ; Fri, 8 Oct 2021 14:52:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 5C43C61039 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 7BA241685; Fri, 8 Oct 2021 16:51:33 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 7BA241685 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1633704743; bh=q+uYn46oKu0+g2FBVUzprQZF2tT9ACUn/Q+MFHy32p8=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=uSFeB2xOSpeFr6BTn8t+2s/xCf+jFJtTqvK9GyYO0yVi3gZTYRgh9HAMNf2KDF296 +go2IYBdFNFMu4e5bXa8KbFkwvwADfA2M/1P+Iktb6q1AcoVJGBE8ZbtqY+BHt+kTq ZGH+qA3EmaAIsAZxddwrakyerlmKJyqbf86l9jgc= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id EBCB4F80229; Fri, 8 Oct 2021 16:51:32 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 95B9BF80246; Fri, 8 Oct 2021 16:51:30 +0200 (CEST) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) (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 E3A42F80229 for ; Fri, 8 Oct 2021 16:51:24 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz E3A42F80229 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="Uf+2BA7a"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="0OTzA8e+" Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id B8A961FF47; Fri, 8 Oct 2021 14:51:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1633704678; 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=OkOCy/4Xaw8Ir/P3EFHrwnlYBBPMrH+frCsht24I4jc=; b=Uf+2BA7a5u9MQ9AwTIaDxuQXFkPo2kkanLAbKsK5vVaHh3pcxkorEvLZ1QjFvUKBzSIOem jcPN1W4W7TlgZDOuBaOvGVtToEVNe+CtU/y7bngoe46OAPRbSOhSo3uR4ivJVCO3BK2QRs sRPznk3Vy2ftXABUYMrpTZL6DIIGhp8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1633704678; 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=OkOCy/4Xaw8Ir/P3EFHrwnlYBBPMrH+frCsht24I4jc=; b=0OTzA8e+LLwyjMsax0yOt8xPhizZ4rq5vmn56CMKBBtOV8c6bSk3OwQx5DkVCNaURVpLSE 41D80stTm3OJApDA== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id DADC4A3B84; Fri, 8 Oct 2021 14:51:17 +0000 (UTC) Date: Fri, 08 Oct 2021 16:51:17 +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: 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> 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 Fri, 08 Oct 2021 16:41:59 +0200, Pierre-Louis Bossart wrote: > > > > >>>> I think the only solution is to follow the example of the PCM case, > >>>> where the type of lock depends on the FE types, with the assumption that > >>>> there are no mixed atomic/non-atomic FE configurations. > >>> > >>> Yes, and I guess we can simply replace those all card->dpcm_lock with > >>> FE's stream lock. It'll solve the atomicity problem, too, and the FE > >>> stream lock can be applied outside the loop of dpcm_be_disconnect() > >>> gracefully. > >>> > >>> And, this should solve the race with dpcm_be_dai_trigger() as well, > >>> because it's called from dpcm_fe_dai_trigger() that is called already > >>> inside the FE's stream lock held by PCM core. A PoC is something like > >>> below. (I replaced the superfluous *_irqsave with *_irq there) > >> > >> No I don't think so. The code starts from an FE and loops for all the > >> BEs connected to that FE, but we want to serialize at the BE level! we > >> really need a dpcm lock at the card level, not the FE/stream level. > > > > The FE lock prevents the race between dpcm_be_dai_trigger() and > > dpcm_be_disconnect(), i.e. the problem Gyeongtaek showed. > > > > The race among concurrent dpcm_be_dai_trigger() calls itself can be > > addressed by BE stream locks as suggested earlier. > > I am not following your line of thought Takashi. > > dpcm_be_disconnect already uses a spin_lock around > > list_del(&dpcm->list_be); > list_del(&dpcm->list_fe); > > and in some other places, are you suggesting we change those to the FE lock? Basically yes. > Otherwise, I understood your proposal as using three locks (existing > spinlock, FE lock, BE lock) to deal with DPCM. If the existing spinlock > and FE lock are combined, we'd still have two locks. Stream locks are more fine-grained, hence more efficient :) The card-level spinlock is superfluous and it can go away. > I was suggesting we use only one ;-) 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. Takashi