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 E1578C433EF for ; Tue, 21 Jun 2022 13:40:11 +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 01B1F20A9; Tue, 21 Jun 2022 15:39:20 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 01B1F20A9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1655818810; bh=fwax/Pkwxrv9EAcaY10xAsTelNY3ww0mq1gyEfcM/5E=; h=Date:Subject:To:References:From:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=c7Zar3jvkx9EG3VIgG3GnXpY/ABcGHsqOI6B5MgWYxylddaE1PSoc9o1JWQXYNWmF jDtIKwG8U5YP8oos+TRTsmc9E69VVUrsiO5NjPhh2r/sqN0/FZ384OlxDktMxAndzm pRzbEyu/+1R/LsT0ClTnmCesTUCLpAwIAL+aLAeU= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 5739CF804F3; Tue, 21 Jun 2022 15:38:50 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 36E48F80155; Tue, 21 Jun 2022 15:38:47 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id D93A6F800CB for ; Tue, 21 Jun 2022 15:38:40 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz D93A6F800CB Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="Ep58IG5Y" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1655818722; x=1687354722; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=fwax/Pkwxrv9EAcaY10xAsTelNY3ww0mq1gyEfcM/5E=; b=Ep58IG5Y6Y88E+og3CxC9iqs5V9HaH2c34ZtpG9ZTshwnyGySFTtABtY RS48lQwRc3NXvq+/6t2CsbgnogKjZnBmfiXQWQaJ8C+Kp/aYCp48sZwNn 6IzsfFgS7XPZvYQAmRSOo3b0yWI9uYida9ITO2x6uHqhMHfJbjw+ic9z6 COUYYYAlnVoRX+mLFAA+7wWmhnYiiiCK0Rx37wzrVaFn2MO2m0rsihnVd wG3dizW0F+dmc7hqIw0Ze0j/G3PkPh7v/qWoumP7a1d5DVFxOzFyY2Uxu n3FSntI4m3WsOj/CA6S/iNC2IcHf/VQ0F717VYIjsEy44+4l8eeIENnFt w==; X-IronPort-AV: E=McAfee;i="6400,9594,10384"; a="280167019" X-IronPort-AV: E=Sophos;i="5.92,209,1650956400"; d="scan'208";a="280167019" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2022 06:38:32 -0700 X-IronPort-AV: E=Sophos;i="5.92,209,1650956400"; d="scan'208";a="591653661" Received: from dpasupul-mobl.amr.corp.intel.com (HELO [10.209.178.35]) ([10.209.178.35]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2022 06:38:31 -0700 Message-ID: <24925485-9d0b-74c3-1e7f-b4906a3314ac@linux.intel.com> Date: Tue, 21 Jun 2022 08:31:53 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0 Thunderbird/91.9.1 Subject: Re: [PATCH] ASoC: core: Exit all links before removing their components Content-Language: en-US To: Cezary Rojewski , alsa-devel@alsa-project.org, broonie@kernel.org References: <20220621115758.3154933-1-cezary.rojewski@intel.com> From: Pierre-Louis Bossart In-Reply-To: <20220621115758.3154933-1-cezary.rojewski@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: hdegoede@redhat.com, amadeuszx.slawinski@linux.intel.com, tiwai@suse.com 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 6/21/22 06:57, Cezary Rojewski wrote: > Flows leading to link->init() and link->exit() are not symmetric. > Currently the relevant part of card probe sequence goes as: > > for_each_card_rtds(card, rtd) > for_each_rtd_components(rtd, i, component) > component->probe() > for_each_card_rtds(card, rtd) > for_each_rtd_dais(rtd, i, dai) > dai->probe() > for_each_card_rtds(card, rtd) > rtd->init() > > On the other side, equivalent remove sequence goes as: > > for_each_card_rtds(card, rtd) > for_each_rtd_dais(rtd, i, dai) > dai->remove() > for_each_card_rtds(card, rtd) > for_each_rtd_components(rtd, i, component) > component->remove() > for_each_card_rtds(card, rtd) > rtd->exit() > > what can lead to errors as link->exit() may still operate on resources > owned by its components despite the probability of them being freed > during the component->remove(). > > This change modifies the remove sequence to: > > for_each_card_rtds(card, rtd) > rtd->exit() > for_each_card_rtds(card, rtd) > for_each_rtd_dais(rtd, i, dai) > dai->remove() > for_each_card_rtds(card, rtd) > for_each_rtd_components(rtd, i, component) > component->remove() > > so code found in link->exit() is safe to touch any component stuff as > component->remove() has not been called yet. This looks harmless as described, but... > > Signed-off-by: Cezary Rojewski > Reviewed-by: Amadeusz Sławiński > --- > sound/soc/soc-core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index 4a3fca50a536..638e781df3b0 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -935,9 +935,6 @@ void snd_soc_remove_pcm_runtime(struct snd_soc_card *card, > { > lockdep_assert_held(&client_mutex); > > - /* release machine specific resources */ > - snd_soc_link_exit(rtd); > - > /* > * Notify the machine driver for extra destruction > */ .... what is not shown here is the soc_free_pcm_runtime(rtd); which will call device_unregister(rtd->dev); .... > @@ -1888,6 +1885,9 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card) > > snd_soc_dapm_shutdown(card); > > + /* release machine specific resources */ > + for_each_card_rtds(card, rtd) > + snd_soc_link_exit(rtd); ... so there's still a risk that the link exit refers to memory that's been released already. We would need to make sure rtd->dev is not used in any of the existing callbacks - or other functions such as e.g. snd_soc_close_delayed_work() which makes use of rtd->dev > /* remove and free each DAI */ > soc_remove_link_dais(card); > soc_remove_link_components(card);