Re: [PATCH 1/2] ASoC: dpcm: fix BE dai not hw_free and shutdown

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1281 bytes --]

On Tue, May 22, 2018 at 05:13:27PM +0800, KaiChieh Chuang wrote:
> In case, one BE is used by two FE1/FE2
> FE1--->BE-->
> FE2--->
> 
> When FE1/FE2 call dpcm_be_dai_hw_free() together
> the BE users will be 2 (> 1), hence cannot be hw_free.
> The be state will leave at, ex. SND_SOC_DPCM_STATE_STOP
> 
> Later FE1/FE2 call dpcm_be_dai_shutdown(),
> will be skip due to wrong state.
> Leaving the BE not being hw_free and shutdown.
> 
> This patch add a flag in snd_soc_dpcm to denote
> the hw_free cannot be excute for this fe->be dpcm.
> The BE dai will be hw_free later when calling
> dpcm_be_dai_shutdown() if still in invalid state.

This works but feels messy and fragile - the problem here is that we use
the users count to decide if we can do a hw_free() but we don't
decrement that users count until shutdown which leaves the race
condition you're fixing here.  We probably need to add a second refcount
here for hw_free() which also feels a bit messy but is probably robust.

Another option is to just unconditionally do the hw_free() and clean up
if we're in the wrong state rather than checking the flag (so basically
your patch but ignoring the flag), that is simpler and should be robust
- I can't think of any reason that'd be a problem?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]