Question regarding delayed trigger in dpcm_set_fe_update_state()

[Cezary Rojewski]

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.


[1]: 
https://lore.kernel.org/all/1415116348-11792-1-git-send-email-tiwai@suse.de/T/


Regards,
Czarek