Re: [RFC PATCH v2 0/5] ASoC: soc-pcm: fix trigger race conditions with shared BE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>>>> 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?

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.

I was suggesting we use only one ;-)




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux