HI, On Tue, Sep 03, 2024 at 11:06:53AM -0700, Allen Pais wrote: > > > >This series of change is inspired by BH workqueue available in recent > >kernel. > > >In Linux FireWire subsystem, tasklet softIRQ context has been utilized to > >operate 1394 OHCI Isochronous Transmit (IT) and Isochronous Receive (IR) > >contexts. The tasklet context is not preferable, as you know. > > >I have already received a proposal[1][2] to replace the usage of tasklet > >with BH workqueue. However, the proposal includes bare replacement for 1394 > >OHCI IT, IR, Asynchronous Transmit (AT), and Asynchronous Receive (AR) > >contexts with neither any care of actual usage for each context nor > >practical test reports. In theory, this kind of change should be done by > >step by step with enough amount of evaluation over software design to avoid > >any disorder. > > >In this series of changes, the usage of tasklet for 1394 OHCI IT/IR > >contexts is just replaced, as a first step. In 1394 OHCI IR/IT events, > >software is expected to process the content of page dedicated to DMA > >transmission for each isochronous context. It means that the content can be > >processed concurrently per isochronous context. Additionally, the content > >of page is immutable as long as the software schedules the transmission > >again for the page. It means that the task to process the content can sleep > >or be preempted. Due to the characteristics, BH workqueue is _not_ used. > > >At present, 1394 OHCI driver is responsible for the maintenance of tasklet > >context, while in this series of change the core function is responsible > >for the maintenance of workqueue and work items. This change is an attempt > >to let each implementation focus on own task. > > >The change affects the following implementations of unit protocols which > >operate isochronous contexts: > > >- firewire-net for IP over 1394 (RFC 2734/3146) > >- firedtv > >- drivers in ALSA firewire stack for IEC 61883-1/6 > >- user space applications > > >As long as reading their codes, the first two drivers look to require no > >change. While the drivers in ALSA firewire stack require change to switch > >the type of context in which callback is executed. The series of change > >includes a patch for them to adapt to work process context. > > >Finally, these changes are tested by devices supported by ALSA firewire > >stack with/without no-period-wakeup runtime of PCM substream. I also tested > >examples in libhinoko[3] as samples of user space applications. Currently I > >face no issue. > > >On the other hand, I have neither tested for firewire-net nor firedtv, > >since I have never used these functions. If someone has any experience to > >use them, I would request to test the change. > > >[1] https://lore.kernel.org/lkml/20240403144558.13398-1-apais@xxxxxxxxxxxxxxxxxxx/ > >[2] https://github.com/allenpais/for-6.9-bh-conversions/issues/1 > >[3] https://git.kernel.org/pub/scm/libs/ieee1394/libhinoko.git/ > > > >Regards > > Thank you for doing this work. You will probably need to send out a v2 > As most of you patches have single line comment instead of Block style > Commnents (/* ..*/). Please have it fixed. Thanks for your comment. However, I think we have a need to update kernel documentation. As long as I know, Mr. Linus has few oposition to C99-style comment[1]. In recent kernel development process, C11 is widely accepted. Actually, we can see many lines with C99-style comment in source tree. For the kernel API documentation, we still need to use Doxygen-like block comment since documentation generator requires it. Additionally we can also see C90 comment is mandatory for UAPI since 'scripts/check-uapi.sh' hard-codes it. For the other part, C99-style comment brings no issue, as long as I know. > - Allen > > >Takashi Sakamoto (5): > > firewire: core: allocate workqueue to handle isochronous contexts in > > card > > firewire: core: add local API for work items scheduled to workqueue > > specific to isochronous contexts > > firewire: ohci: process IT/IR events in sleepable work process to > > obsolete tasklet softIRQ > > firewire: core: non-atomic memory allocation for isochronous event to > > user client > > ALSA: firewire: use nonatomic PCM operation [1] https://lore.kernel.org/lkml/CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_jcbg@xxxxxxxxxxxxxx/ Thanks Takashi Sakamoto
