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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id E6227ECAAA1 for ; Fri, 28 Oct 2022 13:39:49 +0000 (UTC) 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 1F3391F37; Fri, 28 Oct 2022 15:38:57 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 1F3391F37 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1666964387; bh=5g7Y1vloFdbR4H0vylehNDdpmLLq9gt8bbc5NEp3YoY=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=eGRnVHGwcN6pJ8l1H87fsOrwAexYG4zLqhqVT4zVqm5tJRAIJQqSAl2IUEzrEe7yP PluG1SuLCQhr5MdmKNWaAB0DZggJ4lAvJvTVugGbe8t6o18O/cd4RqdFifi/JQtHQ3 YoZeRPpffraxHegPIgaS/46p9QLr/LmyYLUDqhVE= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id A5175F8024C; Fri, 28 Oct 2022 15:38:56 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 1FFD6F800E1; Fri, 28 Oct 2022 15:38:55 +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 ACA45F800E1 for ; Fri, 28 Oct 2022 15:38:51 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz ACA45F800E1 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.b="d+6xTq/A"; dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de header.b="Cn+Fp7UI" Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id A1D8721B12; Fri, 28 Oct 2022 13:38:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1666964331; 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=11q4SPOv3VT37mlDw9oR7280ba9yWG4aeNChi6T5srI=; b=d+6xTq/Ah+lTOmR6lqwLduyg6Wkv7ovc9nb0swq9xLC9klcGv1BHVFYWYAgSrq3r2U3kqE Ird7CBvyhPLZlXpGyRFZWCaYhNOe+T5AvIYdrtzcImPptugVJrb7I/2vbYz2t1v1TRZ4Fo TD060SEMzYzQGQhkYHCWOMaS3AG800c= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1666964331; 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=11q4SPOv3VT37mlDw9oR7280ba9yWG4aeNChi6T5srI=; b=Cn+Fp7UIKc8+QJUVo/WSwLwQxozq54GKcauW8QgPGhIJLkqyfEONFgRdlKruHl6LORU2Bb ZOwo/pjiYzFznrDQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 5F7B61377D; Fri, 28 Oct 2022 13:38:51 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id kOmXFmvbW2OyGQAAMHmgww (envelope-from ); Fri, 28 Oct 2022 13:38:51 +0000 Date: Fri, 28 Oct 2022 15:38:50 +0200 Message-ID: <874jvom5jp.wl-tiwai@suse.de> From: Takashi Iwai To: Cezary Rojewski Subject: Re: Question regarding delayed trigger in dpcm_set_fe_update_state() In-Reply-To: <73e6425f-8e51-e26f-ad83-ccc5df517a43@intel.com> References: <73e6425f-8e51-e26f-ad83-ccc5df517a43@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0 MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Cc: "alsa-devel@alsa-project.org" , Kai Vehmanen , Liam Girdwood , Pierre-Louis Bossart , Takashi Iwai , Hans de Goede , Mark Brown , Ranjani Sridharan , Amadeusz =?ISO-8859-2?Q?S=B3awi?= =?ISO-8859-2?Q?=F1ski?= , =?ISO-8859-1?Q?P?= =?ISO-8859-1?Q?=E9ter?= 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 Wed, 12 Oct 2022 16:07:49 +0200, Cezary Rojewski wrote: > > Hello, > > Writing with question regarding dpcm_set_fe_update_state() function > which is part of sound/soc/soc-pcm.c file and has been introduced with > commit "ASoC: dpcm: Fix race between FE/BE updates and trigger" [1]. > > The part that concerns me is the invocation of > dpcm_fe_dai_do_trigger() regardless of the actual state given DPCM is > in (actual state, not the DPCM_UPDATE_XX). The conditional invocation > of said _trigger() and addition of .trigger_pending field are here to > address a race where PCM state is being modified from multiple > locations simultaneously, at least judging by the commit's > description. > > Note that the dpcm_set_fe_update_state() is called from all the > dai-ops i.e.: startup, shutdown, hw_params, prepare and hw_free. > Now, given that knowledge, we could end up in scenario where > dpcm_fe_dai_do_trigger() is invoked as a part of > dpcm_fe_dai_hw_free(). dpcm_set_fe_update_state() is called there > twice, once with DPCM_UPDATE_FE and once with DPCM_UPDATE_NO. The > second case is the more interesting one since it's called **after** > ->hw_free() callback is invoked for all the DAIs. > > dpcm_fe_dai_hw_free() > dpcm_set_fe_update_state(DPCM_UPDATE_FE) // fine > (...) > dpcm_be_dai_hw_free() // data allocated in hw_params > // is freed here > (...) > dpcm_set_fe_update_state(DPCM_UPDATE_NO) // not fine > > > The last is *not fine* if the .trigger_pending is not a zero, and can > lead to panic as code used during ->trigger() is often manipulating > data allocated during ->hw_params() but that data has just been freed > with ->hw_free(). > > Is what I'm looking at a bug? Or, perhaps there's something I'm > missing in the picture. From my study, it seems that only > dpcm_fe_dai_prepare() is a safe place for calling > dpcm_fe_dai_do_trigger() - once .trigger_pending is taken into > account. Any input is much appreciated. First off, each prepare, hw_params, hw_free, etc call is protected by a mutex, so they can't be run simultaneously. And the commit was only about the (atomic) trigger call during those operations (which may be delayed if happening during other operations). So, the case you suggested in the above can't happen in reality; PCM core won't fire such trigger. Of course, if you observe a race by fuzzing or such, then it'd be worth for investigating further, though. Takashi