Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

Vinod Koul
2024-03-28T05:55:46Z

Hi Allen,

Subsytem is dmaengine, can you rename this to dmaengine: ...

On 27-03-24, 16:03, Allen Pais wrote:
> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.

Thanks for conversion, am happy with BH alternative as it helps in
dmaengine where we need shortest possible time between tasklet and
interrupt handling to maximize dma performance

>
> This patch converts drivers/dma/* from tasklet to BH workqueue.

>
> Based on the work done by Tejun Heo <tj@kernel.org>
> Branch: git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
>
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>
> ---
> drivers/dma/altera-msgdma.c | 15 ++++----
> drivers/dma/apple-admac.c | 15 ++++----
> drivers/dma/at_hdmac.c | 2 +-
> drivers/dma/at_xdmac.c | 15 ++++----
> drivers/dma/bcm2835-dma.c | 2 +-
> drivers/dma/dma-axi-dmac.c | 2 +-
> drivers/dma/dma-jz4780.c | 2 +-
> .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 2 +-
> drivers/dma/dw-edma/dw-edma-core.c | 2 +-
> drivers/dma/dw/core.c | 13 +++----
> drivers/dma/dw/regs.h | 3 +-
> drivers/dma/ep93xx_dma.c | 15 ++++----
> drivers/dma/fsl-edma-common.c | 2 +-
> drivers/dma/fsl-qdma.c | 2 +-
> drivers/dma/fsl_raid.c | 11 +++---
> drivers/dma/fsl_raid.h | 2 +-
> drivers/dma/fsldma.c | 15 ++++----
> drivers/dma/fsldma.h | 3 +-
> drivers/dma/hisi_dma.c | 2 +-
> drivers/dma/hsu/hsu.c | 2 +-
> drivers/dma/idma64.c | 4 +--
> drivers/dma/img-mdc-dma.c | 2 +-
> drivers/dma/imx-dma.c | 27 +++++++-------
> drivers/dma/imx-sdma.c | 6 ++--
> drivers/dma/ioat/dma.c | 17 ++++-----
> drivers/dma/ioat/dma.h | 5 +--
> drivers/dma/ioat/init.c | 2 +-
> drivers/dma/k3dma.c | 19 +++++-----
> drivers/dma/mediatek/mtk-cqdma.c | 35 ++++++++++---------
> drivers/dma/mediatek/mtk-hsdma.c | 2 +-
> drivers/dma/mediatek/mtk-uart-apdma.c | 4 +--
> drivers/dma/mmp_pdma.c | 13 +++----
> drivers/dma/mmp_tdma.c | 11 +++---
> drivers/dma/mpc512x_dma.c | 17 ++++-----
> drivers/dma/mv_xor.c | 13 +++----
> drivers/dma/mv_xor.h | 5 +--
> drivers/dma/mv_xor_v2.c | 23 ++++++------
> drivers/dma/mxs-dma.c | 13 +++----
> drivers/dma/nbpfaxi.c | 15 ++++----
> drivers/dma/owl-dma.c | 2 +-
> drivers/dma/pch_dma.c | 17 ++++-----
> drivers/dma/pl330.c | 31 ++++++++--------
> drivers/dma/plx_dma.c | 13 +++----
> drivers/dma/ppc4xx/adma.c | 17 ++++-----
> drivers/dma/ppc4xx/adma.h | 5 +--
> drivers/dma/pxa_dma.c | 2 +-
> drivers/dma/qcom/bam_dma.c | 35 ++++++++++---------
> drivers/dma/qcom/gpi.c | 18 +++++-----
> drivers/dma/qcom/hidma.c | 11 +++---
> drivers/dma/qcom/hidma.h | 5 +--
> drivers/dma/qcom/hidma_ll.c | 11 +++---
> drivers/dma/qcom/qcom_adm.c | 2 +-
> drivers/dma/sa11x0-dma.c | 27 +++++++-------
> drivers/dma/sf-pdma/sf-pdma.c | 23 ++++++------
> drivers/dma/sf-pdma/sf-pdma.h | 5 +--
> drivers/dma/sprd-dma.c | 2 +-
> drivers/dma/st_fdma.c | 2 +-
> drivers/dma/ste_dma40.c | 17 ++++-----
> drivers/dma/sun6i-dma.c | 33 ++++++++---------
> drivers/dma/tegra186-gpc-dma.c | 2 +-
> drivers/dma/tegra20-apb-dma.c | 19 +++++-----
> drivers/dma/tegra210-adma.c | 2 +-
> drivers/dma/ti/edma.c | 2 +-
> drivers/dma/ti/k3-udma.c | 11 +++---
> drivers/dma/ti/omap-dma.c | 2 +-
> drivers/dma/timb_dma.c | 23 ++++++------
> drivers/dma/txx9dmac.c | 29 +++++++--------
> drivers/dma/txx9dmac.h | 5 +--
> drivers/dma/virt-dma.c | 9 ++---
> drivers/dma/virt-dma.h | 9 ++---
> drivers/dma/xgene-dma.c | 21 +++++------
> drivers/dma/xilinx/xilinx_dma.c | 23 ++++++------
> drivers/dma/xilinx/xilinx_dpdma.c | 21 +++++------
> drivers/dma/xilinx/zynqmp_dma.c | 21 +++++------
> 74 files changed, 442 insertions(+), 395 deletions(-)
>
> diff --git a/drivers/dma/altera-msgdma.c b/drivers/dma/altera-msgdma.c
> index a8e3615235b8..611b5290324b 100644
> --- a/drivers/dma/altera-msgdma.c
> +++ b/drivers/dma/altera-msgdma.c
> @@ -20,6 +20,7 @@
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> #include <linux/of_dma.h>
> +#include <linux/workqueue.h>
>
> #include "dmaengine.h"
>
> @@ -170,7 +171,7 @@ struct msgdma_sw_desc {
> struct msgdma_device {
> spinlock_t lock;
> struct device *dev;
> - struct tasklet_struct irq_tasklet;
> + struct work_struct irq_work;

Can we name these as bh_work to signify that we are always in bh
context? here and everywhere please

> struct list_head pending_list;
> struct list_head free_list;
> struct list_head active_list;
> @@ -676,12 +677,12 @@ static int msgdma_alloc_chan_resources(struct dma_chan *dchan)
> }
>
> /**
> - * msgdma_tasklet - Schedule completion tasklet
> + * msgdma_work - Schedule completion work

...

> @@ -515,7 +516,7 @@ struct gpii {
> enum gpi_pm_state pm_state;
> rwlock_t pm_lock;
> struct gpi_ring ev_ring;
> - struct tasklet_struct ev_task; /* event processing tasklet */
> + struct work_struct ev_task; /* event processing work */
> struct completion cmd_completion;
> enum gpi_cmd gpi_cmd;
> u32 cntxt_type_irq_msk;
> @@ -755,7 +756,7 @@ static void gpi_process_ieob(struct gpii *gpii)
> gpi_write_reg(gpii, gpii->ieob_clr_reg, BIT(0));
>
> gpi_config_interrupts(gpii, MASK_IEOB_SETTINGS, 0);
> - tasklet_hi_schedule(&gpii->ev_task);
> + queue_work(system_bh_highpri_wq, &gpii->ev_task);

This is good conversion, thanks for ensuring system_bh_highpri_wq is
used here
--
~Vinod