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

Arnd Bergmann
2022-05-25T11:09:12Z

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