On Fri, 15 Oct 2021 08:24:41 +0200, Sameer Pujar wrote:
On 10/13/2021 8:00 PM, Pierre-Louis Bossart wrote:
Since the flow for DPCM is based on taking a lock for the FE first, we need to make sure during the connection between a BE and an FE that they both use the same 'atomicity', otherwise we may sleep in atomic context.
If the FE is nonatomic, this patch forces the BE to be nonatomic as well. That should have no negative impact since the BE 'inherits' the FE properties.
However, if the FE is atomic and the BE is not, then the configuration is flagged as invalid.
In normal PCM, atomicity seems to apply only for trigger(). Other callbacks like prepare, hw_params are executed in non-atomic context. So when 'nonatomic' flag is false, still it is possible to sleep in a prepare or hw_param callback and this is true for FE as well. So I am not sure if atomicity is applicable as a whole even for FE.
At this point it does not cause serious problems, but with subsequent patches (especially when patch 7/13 is picked) I see failures. Please refer to patch 7/13 thread for more details.
I am wondering if it is possible to only use locks internally for DPCM state management and decouple BE callbacks from this, like normal PCMs do?
Actually the patch looks like an overkill by adding the FE stream lock at every loop, and this caused the problem, AFAIU.
Basically we need to protect the link addition / deletion while the list traversal (there is a need for protection of BE vs BE access race, but that's a different code path). For the normal cases, it seems already protected by card->pcm_mutex, but the problem is the FE trigger case. It was attempted by dpcm_lock, but that failed because it couldn't be applied properly there.
That said, what we'd need is only: - Drop dpcm_lock codes once - Put FE stream lock around dpcm_be_connect() and dpcm_be_disconnect()
That should suffice for the race at trigger. The FE stream lock is already taken at trigger callback, and the rest list addition / deletion are called from different code paths without stream locks, so the explicit FE stream lock is needed there.
In addition, a lock around dpcm_show_state() might be needed to be replaced with card->pcm_mutex, and we may need to revisit whether all other paths take card->pcm_mutex.
Also, BE-vs-BE race can be protected by taking a BE lock inside dpcm_be_dai_trigger().
Takashi