On Fri, 08 Oct 2021 16:41:59 +0200, Pierre-Louis Bossart wrote: > > > > >>>> I think the only solution is to follow the example of the PCM case, > >>>> where the type of lock depends on the FE types, with the assumption that > >>>> there are no mixed atomic/non-atomic FE configurations. > >>> > >>> Yes, and I guess we can simply replace those all card->dpcm_lock with > >>> FE's stream lock. It'll solve the atomicity problem, too, and the FE > >>> stream lock can be applied outside the loop of dpcm_be_disconnect() > >>> gracefully. > >>> > >>> And, this should solve the race with dpcm_be_dai_trigger() as well, > >>> because it's called from dpcm_fe_dai_trigger() that is called already > >>> inside the FE's stream lock held by PCM core. A PoC is something like > >>> below. (I replaced the superfluous *_irqsave with *_irq there) > >> > >> No I don't think so. The code starts from an FE and loops for all the > >> BEs connected to that FE, but we want to serialize at the BE level! we > >> really need a dpcm lock at the card level, not the FE/stream level. > > > > The FE lock prevents the race between dpcm_be_dai_trigger() and > > dpcm_be_disconnect(), i.e. the problem Gyeongtaek showed. > > > > The race among concurrent dpcm_be_dai_trigger() calls itself can be > > addressed by BE stream locks as suggested earlier. > > I am not following your line of thought Takashi. > > dpcm_be_disconnect already uses a spin_lock around > > list_del(&dpcm->list_be); > list_del(&dpcm->list_fe); > > and in some other places, are you suggesting we change those to the FE lock? Basically yes. > Otherwise, I understood your proposal as using three locks (existing > spinlock, FE lock, BE lock) to deal with DPCM. If the existing spinlock > and FE lock are combined, we'd still have two locks. Stream locks are more fine-grained, hence more efficient :) The card-level spinlock is superfluous and it can go away. > I was suggesting we use only one ;-) Basically we need to protect two things: - The BE links - The concurrent accesses to BEs The former belongs to each FE that holds the links, and the FE stream lock would cover. The latter is rather a per-BE business. An oft-seen risk of multiple locks is deadlocking, but in this case, as long as we keep the lock order FE->BE, nothing wrong can happen. Takashi
