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
