| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240626142146.GK3655345@ragnatech.se>
Date: Wed, 26 Jun 2024 16:21:46 +0200
From: Niklas Söderlund <niklas.soderlund@...natech.se>
To: Wolfram Sang <wsa+renesas@...g-engineering.com>
Cc: linux-renesas-soc@...r.kernel.org, Allen Pais <allen.lkml@...il.com>,
Ulf Hansson <ulf.hansson@...aro.org>, linux-mmc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mmc: sdhi: Convert from tasklet to BH workqueue
Hi Allen and Wolfram,
Nice work.
On 2024-06-26 10:48:21 +0200, Wolfram Sang wrote:
> From: Allen Pais <allen.lkml@...il.com>
>
> 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.
>
> This patch converts the SDHI driver from tasklet to BH workqueue.
>
> Based on the work done by Tejun Heo <tj@...nel.org>
>
> Signed-off-by: Allen Pais <allen.lkml@...il.com>
> [wsa: fixed build faliures, corrected whitespace issues]
> Signed-off-by: Wolfram Sang <wsa+renesas@...g-engineering.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
> ---
>
> Tested on a Renesas Salvator X board with a R-Car M3-W SoC. Same
> performance as with tasklets. Thank you Allen for your work!
>
>
> drivers/mmc/host/renesas_sdhi.h | 4 ++-
> drivers/mmc/host/renesas_sdhi_core.c | 2 ++
> drivers/mmc/host/renesas_sdhi_internal_dmac.c | 26 +++++++++----------
> drivers/mmc/host/renesas_sdhi_sys_dmac.c | 9 +++----
> drivers/mmc/host/tmio_mmc.h | 3 ++-
> drivers/mmc/host/tmio_mmc_core.c | 4 +--
> 6 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
> index 586f94d4dbfd..f12a87442338 100644
> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -11,6 +11,7 @@
>
> #include <linux/dmaengine.h>
> #include <linux/platform_device.h>
> +#include <linux/workqueue.h>
> #include "tmio_mmc.h"
>
> struct renesas_sdhi_scc {
> @@ -67,7 +68,7 @@ struct renesas_sdhi_dma {
> dma_filter_fn filter;
> void (*enable)(struct tmio_mmc_host *host, bool enable);
> struct completion dma_dataend;
> - struct tasklet_struct dma_complete;
> + struct work_struct dma_complete;
> };
>
> struct renesas_sdhi {
> @@ -93,6 +94,7 @@ struct renesas_sdhi {
> unsigned int tap_set;
>
> struct reset_control *rstc;
> + struct tmio_mmc_host *host;
> };
>
> #define host_to_priv(host) \
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 58536626e6c5..04874791541f 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -970,6 +970,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> if (IS_ERR(host))
> return PTR_ERR(host);
>
> + priv->host = host;
> +
> if (of_data) {
> mmc_data->flags |= of_data->tmio_flags;
> mmc_data->ocr_mask = of_data->tmio_ocr_mask;
> diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> index 422fa63a2e99..d4b66daeda66 100644
> --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> @@ -337,7 +337,7 @@ static bool renesas_sdhi_internal_dmac_dma_irq(struct tmio_mmc_host *host)
> writel(status ^ dma_irqs, host->ctl + DM_CM_INFO1);
> set_bit(SDHI_DMA_END_FLAG_DMA, &dma_priv->end_flags);
> if (test_bit(SDHI_DMA_END_FLAG_ACCESS, &dma_priv->end_flags))
> - tasklet_schedule(&dma_priv->dma_complete);
> + queue_work(system_bh_wq, &dma_priv->dma_complete);
> }
>
> return status & dma_irqs;
> @@ -352,7 +352,7 @@ renesas_sdhi_internal_dmac_dataend_dma(struct tmio_mmc_host *host)
> set_bit(SDHI_DMA_END_FLAG_ACCESS, &dma_priv->end_flags);
> if (test_bit(SDHI_DMA_END_FLAG_DMA, &dma_priv->end_flags) ||
> host->data->error)
> - tasklet_schedule(&dma_priv->dma_complete);
> + queue_work(system_bh_wq, &dma_priv->dma_complete);
> }
>
> /*
> @@ -440,9 +440,9 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host,
> renesas_sdhi_internal_dmac_enable_dma(host, false);
> }
>
> -static void renesas_sdhi_internal_dmac_issue_tasklet_fn(unsigned long arg)
> +static void renesas_sdhi_internal_dmac_issue_work_fn(struct work_struct *work)
> {
> - struct tmio_mmc_host *host = (struct tmio_mmc_host *)arg;
> + struct tmio_mmc_host *host = from_work(host, work, dma_issue);
> struct renesas_sdhi *priv = host_to_priv(host);
>
> tmio_mmc_enable_mmc_irqs(host, TMIO_STAT_DATAEND);
> @@ -454,7 +454,7 @@ static void renesas_sdhi_internal_dmac_issue_tasklet_fn(unsigned long arg)
> /* on CMD errors, simulate DMA end immediately */
> set_bit(SDHI_DMA_END_FLAG_DMA, &priv->dma_priv.end_flags);
> if (test_bit(SDHI_DMA_END_FLAG_ACCESS, &priv->dma_priv.end_flags))
> - tasklet_schedule(&priv->dma_priv.dma_complete);
> + queue_work(system_bh_wq, &priv->dma_priv.dma_complete);
> }
> }
>
> @@ -484,9 +484,11 @@ static bool renesas_sdhi_internal_dmac_complete(struct tmio_mmc_host *host)
> return true;
> }
>
> -static void renesas_sdhi_internal_dmac_complete_tasklet_fn(unsigned long arg)
> +static void renesas_sdhi_internal_dmac_complete_work_fn(struct work_struct *work)
> {
> - struct tmio_mmc_host *host = (struct tmio_mmc_host *)arg;
> + struct renesas_sdhi_dma *dma_priv = from_work(dma_priv, work, dma_complete);
> + struct renesas_sdhi *priv = container_of(dma_priv, typeof(*priv), dma_priv);
> + struct tmio_mmc_host *host = priv->host;
>
> spin_lock_irq(&host->lock);
> if (!renesas_sdhi_internal_dmac_complete(host))
> @@ -544,12 +546,10 @@ renesas_sdhi_internal_dmac_request_dma(struct tmio_mmc_host *host,
> /* Each value is set to non-zero to assume "enabling" each DMA */
> host->chan_rx = host->chan_tx = (void *)0xdeadbeaf;
>
> - tasklet_init(&priv->dma_priv.dma_complete,
> - renesas_sdhi_internal_dmac_complete_tasklet_fn,
> - (unsigned long)host);
> - tasklet_init(&host->dma_issue,
> - renesas_sdhi_internal_dmac_issue_tasklet_fn,
> - (unsigned long)host);
> + INIT_WORK(&priv->dma_priv.dma_complete,
> + renesas_sdhi_internal_dmac_complete_work_fn);
> + INIT_WORK(&host->dma_issue,
> + renesas_sdhi_internal_dmac_issue_work_fn);
>
> /* Add pre_req and post_req */
> host->ops.pre_req = renesas_sdhi_internal_dmac_pre_req;
> diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> index 9cf7f9feab72..5a6f41318645 100644
> --- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> @@ -312,9 +312,9 @@ static void renesas_sdhi_sys_dmac_start_dma(struct tmio_mmc_host *host,
> }
> }
>
> -static void renesas_sdhi_sys_dmac_issue_tasklet_fn(unsigned long priv)
> +static void renesas_sdhi_sys_dmac_issue_work_fn(struct work_struct *work)
> {
> - struct tmio_mmc_host *host = (struct tmio_mmc_host *)priv;
> + struct tmio_mmc_host *host = from_work(host, work, dma_issue);
> struct dma_chan *chan = NULL;
>
> spin_lock_irq(&host->lock);
> @@ -401,9 +401,8 @@ static void renesas_sdhi_sys_dmac_request_dma(struct tmio_mmc_host *host,
> goto ebouncebuf;
>
> init_completion(&priv->dma_priv.dma_dataend);
> - tasklet_init(&host->dma_issue,
> - renesas_sdhi_sys_dmac_issue_tasklet_fn,
> - (unsigned long)host);
> + INIT_WORK(&host->dma_issue,
> + renesas_sdhi_sys_dmac_issue_work_fn);
> }
>
> renesas_sdhi_sys_dmac_enable_dma(host, true);
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 2af5730c21f4..a75755f31d31 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -21,6 +21,7 @@
> #include <linux/scatterlist.h>
> #include <linux/spinlock.h>
> #include <linux/interrupt.h>
> +#include <linux/workqueue.h>
>
> #define CTL_SD_CMD 0x00
> #define CTL_ARG_REG 0x04
> @@ -153,7 +154,7 @@ struct tmio_mmc_host {
> bool dma_on;
> struct dma_chan *chan_rx;
> struct dma_chan *chan_tx;
> - struct tasklet_struct dma_issue;
> + struct work_struct dma_issue;
> struct scatterlist bounce_sg;
> u8 *bounce_buf;
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 2780f0a29871..b61a6310311d 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -608,7 +608,7 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, unsigned int stat)
> } else {
> tmio_mmc_disable_mmc_irqs(host,
> TMIO_MASK_READOP);
> - tasklet_schedule(&host->dma_issue);
> + queue_work(system_bh_wq, &host->dma_issue);
> }
> } else {
> if (!host->dma_on) {
> @@ -616,7 +616,7 @@ static void tmio_mmc_cmd_irq(struct tmio_mmc_host *host, unsigned int stat)
> } else {
> tmio_mmc_disable_mmc_irqs(host,
> TMIO_MASK_WRITEOP);
> - tasklet_schedule(&host->dma_issue);
> + queue_work(system_bh_wq, &host->dma_issue);
> }
> }
> } else {
> --
> 2.43.0
>
>
--
Kind Regards,
Niklas Söderlund
Powered by blists - more mailing lists