Re: [RFC 1/1] drivers/dma/*: replace tasklets with workqueue

[Arnd Bergmann <arnd@arndb.de>]

On Wed, May 25, 2022 at 11:24 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Apr 19, 2022 at 11:17 PM Allen Pais <apais@linux.microsoft.com> wrote:
>
> > The tasklet is an old API which will be deprecated, workqueue API
> > cab be used instead of them.
> >
> > This patch replaces the tasklet usage in drivers/dma/* with a
> > simple work.
> >
> > Github: https://github.com/KSPP/linux/issues/94
> >
> > Signed-off-by: Allen Pais <apais@linux.microsoft.com>
>
> Paging Vincent Guittot and Arnd Bergmann on the following question
> on this patch set:
>
> - Will replacing tasklets with workque like this negatively impact the
>   performance on DMA engine bottom halves?

I think it will in some cases but not others. The problem I see is that
the short patch description makes it sound like a trivial conversion of a
single subsystem, but in reality this interacts with all the drivers using
DMA engines, including tty/serial, sound, mmc and spi.

In many cases, the change is an improvement, but I can see a number
of ways this might go wrong:

- for audio, waiting to schedule the workqueue task may add enough
  latency to lead to audible skips

- for serial, transferring a few characters through DMA is probably
  more expensive now than using MMIO, which might mean that
  there may no longer be a point in using DMA in the first place.

- Some drivers such as dw_mmc schedule another tasklet from the
  callback. If the tasklet is turned into a workqueue, this becomes
  a bit pointless unless we change the called drivers first.

What might work better in the case of the dmaengine API would
be an approach like:

1. add helper functions to call the callback functions from a
    tasklet locally defined in drivers/dma/dmaengine.c to allow
    deferring it from hardirq context

2. Change all  tasklets that are not part of the callback
    mechanism to work queue functions, I only see
    xilinx_dpdma_chan_err_task in the patch, but there
    may be more

3. change all drivers to move their custom tasklets back into
    hardirq context and instead call the new helper for deferring
    the callback.

4. Extend the dmaengine callback API to let slave drivers
    pick hardirq, tasklet or task context for the callback.
    task context can mean either a workqueue, or a threaded
    IRQ here, with the default remaining the tasklet version.

5. Change slave drivers to pick either hardirq or task context
    depending on their requirements

6. Remove the tasklet version.

This is of course a lot more complicated than Allen's
approach, but I think the end result would be much better.

         Arnd