Re: [RFC PATCH 1/1] dmaengine: introduce dmaengine_bh_wq and bh helpers

["Arnd Bergmann" <arnd@arndb.de>]

On Thu, Jan 8, 2026, at 20:22, Allen wrote:
>
>    Thanks for the feedback. My intent with WQ_BH was to keep callbacks in
>   softirq/BH context, but I agree the scheduling overhead and existing tasklet
>   assumptions are valid concerns.

Hi Allen,

Sorry about missing the bit about WQ_BH, I forgot about the details
of or previous discussion and this is pretty much what I had
suggested last time, 

>   I can re-spin the RFC to drop the workqueue entirely and keep
> tasklet semantics,
>   while still abstracting tasklet handling into dmaengine helpers so drivers no
>   longer directly manipulate tasklets. That keeps
> dmaengine_desc_callback_invoke()
>   in tasklet context and avoids breaking DMA users that rely on that behavior.

It's probably fine to do both, but a series of two patches (first introduce
the per-channel API, then move it over to WQ_BH) may be slightly
clearer here.

I'm not sure why the dmaengine_*_bh_wq() functions are exported
interfaces, as far as I can tell, you use them only internally
in the dma_chan_*_bh() functions, so making them static would
let the compiler inline them where possible.

There are of course dmaengine drivers that use tasklets for other
purposes than the channel callback. Was your idea here to use
the same workqueue for these? I would perhaps hold off on that for
the moment and see if there is a better alternative for those,
possibly hardirq context, threaded irq or a regular workqueue
depending on the driver.

>   A hardirq callback path feels like a larger API change, so I’d
> prefer to handle that as a separate follow‑up (e.g. explicit hardirq
>  callback/flag + user migration where safe). Thoughts?

Yes, definitely keep that separate. I still think that this is
what we want eventually for a bigger improvement, but your
patch seems valuable on its own as well.

Some more thoughts on where that later change could take us:

>    /*
>   - * This tasklet handles the completion of a DMA descriptor by
>   - * calling its callback and freeing it.
>   + * This bottom-half handler completes a DMA descriptor by invoking its
>   + * callback and freeing it.
>     */
>   -static void vchan_complete(struct tasklet_struct *t)
>   +static void vchan_complete(struct dma_chan *chan)
>    {
>   -    struct virt_dma_chan *vc = from_tasklet(vc, t, task);
>   +    struct virt_dma_chan *vc = to_virt_chan(chan);
>        struct virt_dma_desc *vd, *_vd;
>        struct dmaengine_desc_callback cb;
>        LIST_HEAD(head);
>   @@ -131,7 +131,7 @@ void vchan_init(struct virt_dma_chan *vc, struct
> dma_device *dmadev)
>        INIT_LIST_HEAD(&vc->desc_completed);
>        INIT_LIST_HEAD(&vc->desc_terminated);
>
>   -    tasklet_setup(&vc->task, vchan_complete);
>   +    dma_chan_init_bh(&vc->chan, vchan_complete);

This is where I think it makes sense to start, again for
the vchan imlmenentation. What the dmaengine drivers have
is a per-driver tasklet (or WQ_BH) with a single complete()
function that directly calls into the client drivers for
each completion that has happened.

Since the context we want depends on the type of client
driver, I think a good approach would be to start
by modifying vchan_cookie_complete() to allow it to
call the callback function directly from hardirq context
when the client asks for that, and avoid the round trip
through the tasklet where possible.

A new bit in the dma_ctrl_flags word could be used
to ask for hardirq vs softirq context, and the existing
drivers just fall back to using a tasklet for softirq
context.

      Arnd