Re: [alsa-devel] [PATCH 0/6] ALSA: firewire: handle several IR/IT context in callback of the IT context

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi,

On Sun, Oct 20, 2019 at 05:37:19PM +0900, Takashi Sakamoto wrote:
> On Sat, Oct 19, 2019 at 09:22:16AM +0200, Takashi Iwai wrote:
> > Although the preempt handling in AMDTP looks a bit suspicious, it
> > should be OK as long as the code has been tested, so I took as is.
> 
> I can understand your concern but it works well as long as I tested.
> In this time I use preemption-disable during processing queued packet in
> process context but before I had used irq-disable instead.
> 
> I depict a call graph to process isoc packet in process context below:
> 
> On pcm.ack() or pcm.pointer() callbacks:
> fw_iso_context_flush_completions()
> ->struct fw_card_driver.flush_iso_completions()
> = ohci_flush_iso_completions()
>   ->tasklet_disable()
>   ->context_tasklet()
>     (read registers on 1394 OHCI controller to get info for queued packet)
>   ->flush_iso_completions()
>     ->struct fw_iso_context.callback.sc()
>     = amdtp_master_callback()
>       ->out_stream_callback() (for irq-target)
>       ->fw_iso_context_flush_completions()
>         ->...
>           ->out_stream_callback() or in_stream_callback() (for non irq-target)
>   ->tasklet_enable()
> 
> The tasklet of IT context for irq-target AMDTP stream is temporarily
> disabled when flushing isoc packets queued for the context. The tasklet
> doesn't run even if it's queued in hw IRQ context. In a point to
> processing queued packet for several isoc contexts in the same domain,
> I have no concern without any irq-flag handling for local cpu.
> 
> 1394 OHCI controller still continue isoc cycle during the above call
> graph (= 125 usec interval according to 24.576 MHz clock). Actually the
> number of queued packets for non-irq-target AMDTP stream can be slightly
> different from the number for irq-target AMDTP stream by one or two
> packets without any interruption. In a case that any interruption occurs
> after processing queued packets for the irq-target stream, it's likely to
> process largely different number of queued packets for the rest of AMDTP
> streams in the same domain after resumed. It's desirable not to make such
> count gap between streams in the same domain and I leave it for my future
> work.
> 
> In this time the count gap is allowed. I use kernel preemption to avoid
> the interruption but to catch hw IRQ from 1394 OHCI controller (and the
> other hardware).
> 
> 
> In another point, there's a race condition against flushing queued packet
> in runtime between several PCM substreams for AMDTP streams in the same
> domain. ALSA PCM core executes pcm.pointer and pcm.ack callback under spin
> lock of runtime of PCM substream, thus the race is controlled for operations
> to single PCM substream. However some PCM substreams are associated to the
> same domain via AMDTP streams. At present I never add any solution for this
> race.

I realize that this race is managed as well, by a call of
test_and_set_bit_lock(). When operations for several PCM substream call
pcm.pointer or pcm.ack simultaneously, one of them can flush queued
packets. I refine the above call graph:

On pcm.ack() or pcm.pointer() callbacks:
fw_iso_context_flush_completions()
->struct fw_card_driver.flush_iso_completions()
= ohci_flush_iso_completions()
  ->tasklet_disable()
  if (!test_and_set_bit_lock())
    ->context_tasklet()
      (read registers on 1394 OHCI controller to get info for queued packet)
    ->flush_iso_completions()
      ->struct fw_iso_context.callback.sc()
      = amdtp_master_callback()
        ->out_stream_callback() (for irq-target)
        ->fw_iso_context_flush_completions()
          ->...
            ->out_stream_callback() or in_stream_callback() (for non irq-target)
    ->clear_bit_unlock()
  ->tasklet_enable()


Regards

Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel