All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Sameer Pujar <spujar@nvidia.com>, Takashi Iwai <tiwai@suse.com>,
	open list <linux-kernel@vger.kernel.org>,
	vkoul@kernel.org, broonie@kernel.org,
	Gyeongtaek Lee <gt82.lee@samsung.com>,
	Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Subject: Re: [RFC PATCH v3 05/13] ASoC: soc-pcm: align BE 'atomicity' with that of the FE
Date: Fri, 15 Oct 2021 12:08:48 -0500	[thread overview]
Message-ID: <c0803288-efb1-aaeb-218f-e1a6ba87abd7@linux.intel.com> (raw)
In-Reply-To: <s5hmtnateeo.wl-tiwai@suse.de>


>> I have not been able to figure out when you need
>> a) the pcm_mutex only
>> b) the fe stream lock only
>> c) both pcm_mutex and fe stream lock
> 
> The pcm_mutex is needed for every sleepable function that treat DPCM
> FE link, but the mutex is taken only at the upper level, i.e. the
> top-most caller like PCM ops FE itself or the DAPM calls.
> 
> That said, pcm_mutex is the top-most protection of BE links in FE.
> But, there is a code path where a mutex can't be used, and that's the
> FE and BE trigger.  For protecting against this, the FE stream lock is
> taken only at the placing both adding and deleting a BE *in addition*.
> At those places, both pcm_mutex and FE stream lock are taken.
> 
> BE stream lock is taken in addition below the above mutex and FE
> locks.

Thanks Takashi, now I get the idea. Makes sense indeed. I'll make sure
to add this explanation to the commit message/cover so that it's not lost.

I added your three patches to my tests, so far so good, code is that
https://github.com/thesofproject/linux/pull/3215

Thanks and have a nice week-end.
-Pierre

WARNING: multiple messages have this Message-ID (diff)
From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	open list <linux-kernel@vger.kernel.org>,
	Sameer Pujar <spujar@nvidia.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Takashi Iwai <tiwai@suse.com>,
	vkoul@kernel.org, broonie@kernel.org,
	Gyeongtaek Lee <gt82.lee@samsung.com>,
	Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Subject: Re: [RFC PATCH v3 05/13] ASoC: soc-pcm: align BE 'atomicity' with that of the FE
Date: Fri, 15 Oct 2021 12:08:48 -0500	[thread overview]
Message-ID: <c0803288-efb1-aaeb-218f-e1a6ba87abd7@linux.intel.com> (raw)
In-Reply-To: <s5hmtnateeo.wl-tiwai@suse.de>


>> I have not been able to figure out when you need
>> a) the pcm_mutex only
>> b) the fe stream lock only
>> c) both pcm_mutex and fe stream lock
> 
> The pcm_mutex is needed for every sleepable function that treat DPCM
> FE link, but the mutex is taken only at the upper level, i.e. the
> top-most caller like PCM ops FE itself or the DAPM calls.
> 
> That said, pcm_mutex is the top-most protection of BE links in FE.
> But, there is a code path where a mutex can't be used, and that's the
> FE and BE trigger.  For protecting against this, the FE stream lock is
> taken only at the placing both adding and deleting a BE *in addition*.
> At those places, both pcm_mutex and FE stream lock are taken.
> 
> BE stream lock is taken in addition below the above mutex and FE
> locks.

Thanks Takashi, now I get the idea. Makes sense indeed. I'll make sure
to add this explanation to the commit message/cover so that it's not lost.

I added your three patches to my tests, so far so good, code is that
https://github.com/thesofproject/linux/pull/3215

Thanks and have a nice week-end.
-Pierre

  reply	other threads:[~2021-10-15 17:10 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-13 14:30 [RFC PATCH v3 00/13] ASoC : soc-pcm: fix trigger race conditions with shared BE Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 01/13] ASoC: soc-pcm: remove snd_soc_dpcm_fe_can_update() Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 02/13] ASoC: soc-pcm: don't export local functions, use static Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 03/13] ASoC: soc-pcm: use proper indentation on 'continue' Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 04/13] ASoC: soc-pcm: introduce snd_soc_dpcm_fe_lock_irq/unlock_irq() Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-15  6:24   ` Sameer Pujar
2021-10-15  6:24     ` Sameer Pujar
2021-10-15 12:24     ` Pierre-Louis Bossart
2021-10-15 12:24       ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 05/13] ASoC: soc-pcm: align BE 'atomicity' with that of the FE Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-15  6:24   ` Sameer Pujar
2021-10-15  6:24     ` Sameer Pujar
2021-10-15  7:39     ` Takashi Iwai
2021-10-15  7:39       ` Takashi Iwai
2021-10-15 11:22       ` Pierre-Louis Bossart
2021-10-15 11:22         ` Pierre-Louis Bossart
2021-10-15 12:04         ` Pierre-Louis Bossart
2021-10-15 12:04           ` Pierre-Louis Bossart
2021-10-15 15:38         ` Takashi Iwai
2021-10-15 15:38           ` Takashi Iwai
2021-10-15 16:22           ` Pierre-Louis Bossart
2021-10-15 16:22             ` Pierre-Louis Bossart
2021-10-15 16:56             ` Takashi Iwai
2021-10-15 16:56               ` Takashi Iwai
2021-10-15 17:08               ` Pierre-Louis Bossart [this message]
2021-10-15 17:08                 ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 06/13] ASoC: soc-pcm: remove dpcm spin_lock, use PCM stream lock Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 07/13] ASoC: soc-pcm: protect for_each_dpcm_be() loops Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-15  6:24   ` Sameer Pujar
2021-10-15  6:24     ` Sameer Pujar
2021-10-15 11:02     ` Pierre-Louis Bossart
2021-10-15 11:02       ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 08/13] ASoC: soc-compress: " Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 09/13] ASoC: sh: rcar: " Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 10/13] ASoC: fsl: asrc_dma: " Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 11/13] ASoC: soc-pcm: serialize BE triggers Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 12/13] ASoC: soc-pcm: test refcount before triggering Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart
2021-10-13 14:30 ` [RFC PATCH v3 13/13] ASoC: soc-pcm: fix BE handling of PAUSE_RELEASE Pierre-Louis Bossart
2021-10-13 14:30   ` Pierre-Louis Bossart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c0803288-efb1-aaeb-218f-e1a6ba87abd7@linux.intel.com \
    --to=pierre-louis.bossart@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=gt82.lee@samsung.com \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter.ujfalusi@linux.intel.com \
    --cc=spujar@nvidia.com \
    --cc=tiwai@suse.com \
    --cc=tiwai@suse.de \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.