On Mon, Jul 29, 2024 at 10:16:04AM +0000, edmund.raile wrote: > > $ git revert -s b5b519965c4c > Yes, 5b5 can be removed via revert, but what is the difference in > effect? Just time saving? The title of commit generated by git-revert could be helpful when reading history of changes later. The code change is itself important, while the history of changes often assists developers to work between several trees. > > $ git revert -s 7ba5ca32fe6e > This one I'd like to ask you about: > The original inline comment in amdtp-stream.c > amdtp_domain_stream_pcm_pointer() > ``` > // This function is called in software IRQ context of > // period_work or process context. > // > // When the software IRQ context was scheduled by software IRQ > // context of IT contexts, queued packets were already handled. > // Therefore, no need to flush the queue in buffer furthermore. > // > // When the process context reach here, some packets will be > // already queued in the buffer. These packets should be handled > // immediately to keep better granularity of PCM pointer. > // > // Later, the process context will sometimes schedules software > // IRQ context of the period_work. Then, no need to flush the > // queue by the same reason as described in the above > ``` > (let's call the above v1) was replaced with > ``` > // In software IRQ context, the call causes dead-lock to disable the tasklet > // synchronously. > ``` > on occasion of 7ba5ca32fe6e (let's call this v2). > > I sought to replace it with > ``` > // use wq to prevent deadlock between process context spin_lock > // of snd_pcm_stream_lock_irq() in snd_pcm_status64() and > // softIRQ context spin_lock of snd_pcm_stream_lock_irqsave() > // in snd_pcm_period_elapsed() > ``` > to prevent this issue from occurring again (let's call this v3). > > Should I include v1, v3 or a combination of v1 and v3 in my next patch? I think we pay enough attention to regenerate the deadlock, thus v3 is a good choice. But the cause of deadlock in the above description is a bit wrong. It is typical AB/BA deadlock, like: Thread 0: 1. Acquire stream lock by calling 'snd_pcm_stream_lock_irq()' or so 2. Wait until running tasklet finishes by calling 'tasklet_disable_in_atomic()' (actually at tasklet_unlock_spin_wait()) Thread 1: 1. Launch tasklet 2. Wait until the stream lock released The softIRQ context does not related to any type of lock in sound subsystem essentially. Thanks Takashi Sakamoto
