[PATCHv2] mmc: meson: remove redundant mmc_request_done() call from irq context

Message ID 20230607082713.517157-1-martin@geanix.com
State New
Headers
Series [PATCHv2] mmc: meson: remove redundant mmc_request_done() call from irq context |

Commit Message

Martin Hundebøll June 7, 2023, 8:27 a.m. UTC
  The call to mmc_request_done() can schedule, so it must not be called
from irq context. Wake the irq thread if it needs to be called, and let
its existing logic do its work.

Fixes the following kernel bug, which appears when running an RT patched
kernel on the AmLogic Meson AXG A113X SoC:
[   11.111407] BUG: scheduling while atomic: kworker/0:1H/75/0x00010001
[   11.111438] Modules linked in:
[   11.111451] CPU: 0 PID: 75 Comm: kworker/0:1H Not tainted 6.4.0-rc3-rt2-rtx-00081-gfd07f41ed6b4-dirty #1
[   11.111461] Hardware name: RTX AXG A113X Linux Platform Board (DT)
[   11.111469] Workqueue: kblockd blk_mq_run_work_fn
[   11.111492] Call trace:
[   11.111497]  dump_backtrace+0xac/0xe8
[   11.111510]  show_stack+0x18/0x28
[   11.111518]  dump_stack_lvl+0x48/0x60
[   11.111530]  dump_stack+0x18/0x24
[   11.111537]  __schedule_bug+0x4c/0x68
[   11.111548]  __schedule+0x80/0x574
[   11.111558]  schedule_loop+0x2c/0x50
[   11.111567]  schedule_rtlock+0x14/0x20
[   11.111576]  rtlock_slowlock_locked+0x468/0x730
[   11.111587]  rt_spin_lock+0x40/0x64
[   11.111596]  __wake_up_common_lock+0x5c/0xc4
[   11.111610]  __wake_up+0x18/0x24
[   11.111620]  mmc_blk_mq_req_done+0x68/0x138
[   11.111633]  mmc_request_done+0x104/0x118
[   11.111644]  meson_mmc_request_done+0x38/0x48
[   11.111654]  meson_mmc_irq+0x128/0x1f0
[   11.111663]  __handle_irq_event_percpu+0x70/0x114
[   11.111674]  handle_irq_event_percpu+0x18/0x4c
[   11.111683]  handle_irq_event+0x80/0xb8
[   11.111691]  handle_fasteoi_irq+0xa4/0x120
[   11.111704]  handle_irq_desc+0x20/0x38
[   11.111712]  generic_handle_domain_irq+0x1c/0x28
[   11.111721]  gic_handle_irq+0x8c/0xa8
[   11.111735]  call_on_irq_stack+0x24/0x4c
[   11.111746]  do_interrupt_handler+0x88/0x94
[   11.111757]  el1_interrupt+0x34/0x64
[   11.111769]  el1h_64_irq_handler+0x18/0x24
[   11.111779]  el1h_64_irq+0x64/0x68
[   11.111786]  __add_wait_queue+0x0/0x4c
[   11.111795]  mmc_blk_rw_wait+0x84/0x118
[   11.111804]  mmc_blk_mq_issue_rq+0x5c4/0x654
[   11.111814]  mmc_mq_queue_rq+0x194/0x214
[   11.111822]  blk_mq_dispatch_rq_list+0x3ac/0x528
[   11.111834]  __blk_mq_sched_dispatch_requests+0x340/0x4d0
[   11.111847]  blk_mq_sched_dispatch_requests+0x38/0x70
[   11.111858]  blk_mq_run_work_fn+0x3c/0x70
[   11.111865]  process_one_work+0x17c/0x1f0
[   11.111876]  worker_thread+0x1d4/0x26c
[   11.111885]  kthread+0xe4/0xf4
[   11.111894]  ret_from_fork+0x10/0x20

Fixes: 51c5d8447bd7 ("MMC: meson: initial support for GX platforms")
Cc: stable@vger.kernel.org
Signed-off-by: Martin Hundebøll <martin@geanix.com>
---
Version 1 of this patch:
https://lore.kernel.org/linux-amlogic/20230606065918.460866-1-martin@geanix.com/

Changes since v1:
 * remove redundant change to meson_mmc_irq_thread(), as per Martin's
   review
 * return early instead of assigning to "ret" variable
 * change commit short-log to reflect code removal instead of it being
   moved. (Was: "mmc: meson: move mmc_request_done() call to irq thread")

 drivers/mmc/host/meson-gx-mmc.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
  

Comments

Ulf Hansson June 14, 2023, 10:31 a.m. UTC | #1
On Wed, 7 Jun 2023 at 10:27, Martin Hundebøll <martin@geanix.com> wrote:
>
> The call to mmc_request_done() can schedule, so it must not be called
> from irq context. Wake the irq thread if it needs to be called, and let
> its existing logic do its work.
>
> Fixes the following kernel bug, which appears when running an RT patched
> kernel on the AmLogic Meson AXG A113X SoC:
> [   11.111407] BUG: scheduling while atomic: kworker/0:1H/75/0x00010001
> [   11.111438] Modules linked in:
> [   11.111451] CPU: 0 PID: 75 Comm: kworker/0:1H Not tainted 6.4.0-rc3-rt2-rtx-00081-gfd07f41ed6b4-dirty #1
> [   11.111461] Hardware name: RTX AXG A113X Linux Platform Board (DT)
> [   11.111469] Workqueue: kblockd blk_mq_run_work_fn
> [   11.111492] Call trace:
> [   11.111497]  dump_backtrace+0xac/0xe8
> [   11.111510]  show_stack+0x18/0x28
> [   11.111518]  dump_stack_lvl+0x48/0x60
> [   11.111530]  dump_stack+0x18/0x24
> [   11.111537]  __schedule_bug+0x4c/0x68
> [   11.111548]  __schedule+0x80/0x574
> [   11.111558]  schedule_loop+0x2c/0x50
> [   11.111567]  schedule_rtlock+0x14/0x20
> [   11.111576]  rtlock_slowlock_locked+0x468/0x730
> [   11.111587]  rt_spin_lock+0x40/0x64
> [   11.111596]  __wake_up_common_lock+0x5c/0xc4
> [   11.111610]  __wake_up+0x18/0x24
> [   11.111620]  mmc_blk_mq_req_done+0x68/0x138
> [   11.111633]  mmc_request_done+0x104/0x118
> [   11.111644]  meson_mmc_request_done+0x38/0x48
> [   11.111654]  meson_mmc_irq+0x128/0x1f0
> [   11.111663]  __handle_irq_event_percpu+0x70/0x114
> [   11.111674]  handle_irq_event_percpu+0x18/0x4c
> [   11.111683]  handle_irq_event+0x80/0xb8
> [   11.111691]  handle_fasteoi_irq+0xa4/0x120
> [   11.111704]  handle_irq_desc+0x20/0x38
> [   11.111712]  generic_handle_domain_irq+0x1c/0x28
> [   11.111721]  gic_handle_irq+0x8c/0xa8
> [   11.111735]  call_on_irq_stack+0x24/0x4c
> [   11.111746]  do_interrupt_handler+0x88/0x94
> [   11.111757]  el1_interrupt+0x34/0x64
> [   11.111769]  el1h_64_irq_handler+0x18/0x24
> [   11.111779]  el1h_64_irq+0x64/0x68
> [   11.111786]  __add_wait_queue+0x0/0x4c
> [   11.111795]  mmc_blk_rw_wait+0x84/0x118
> [   11.111804]  mmc_blk_mq_issue_rq+0x5c4/0x654
> [   11.111814]  mmc_mq_queue_rq+0x194/0x214
> [   11.111822]  blk_mq_dispatch_rq_list+0x3ac/0x528
> [   11.111834]  __blk_mq_sched_dispatch_requests+0x340/0x4d0
> [   11.111847]  blk_mq_sched_dispatch_requests+0x38/0x70
> [   11.111858]  blk_mq_run_work_fn+0x3c/0x70
> [   11.111865]  process_one_work+0x17c/0x1f0
> [   11.111876]  worker_thread+0x1d4/0x26c
> [   11.111885]  kthread+0xe4/0xf4
> [   11.111894]  ret_from_fork+0x10/0x20
>
> Fixes: 51c5d8447bd7 ("MMC: meson: initial support for GX platforms")
> Cc: stable@vger.kernel.org
> Signed-off-by: Martin Hundebøll <martin@geanix.com>

Applied for fixes, thanks!

Kind regards
Uffe


> ---
> Version 1 of this patch:
> https://lore.kernel.org/linux-amlogic/20230606065918.460866-1-martin@geanix.com/
>
> Changes since v1:
>  * remove redundant change to meson_mmc_irq_thread(), as per Martin's
>    review
>  * return early instead of assigning to "ret" variable
>  * change commit short-log to reflect code removal instead of it being
>    moved. (Was: "mmc: meson: move mmc_request_done() call to irq thread")
>
>  drivers/mmc/host/meson-gx-mmc.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index b8514d9d5e736..f90b0fd8d8b00 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -991,11 +991,8 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>
>                 if (data && !cmd->error)
>                         data->bytes_xfered = data->blksz * data->blocks;
> -               if (meson_mmc_bounce_buf_read(data) ||
> -                   meson_mmc_get_next_command(cmd))
> -                       ret = IRQ_WAKE_THREAD;
> -               else
> -                       ret = IRQ_HANDLED;
> +
> +               return IRQ_WAKE_THREAD;
>         }
>
>  out:
> @@ -1007,9 +1004,6 @@ static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
>                 writel(start, host->regs + SD_EMMC_START);
>         }
>
> -       if (ret == IRQ_HANDLED)
> -               meson_mmc_request_done(host->mmc, cmd->mrq);
> -
>         return ret;
>  }
>
> --
> 2.40.1
>
  

Patch

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index b8514d9d5e736..f90b0fd8d8b00 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -991,11 +991,8 @@  static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 
 		if (data && !cmd->error)
 			data->bytes_xfered = data->blksz * data->blocks;
-		if (meson_mmc_bounce_buf_read(data) ||
-		    meson_mmc_get_next_command(cmd))
-			ret = IRQ_WAKE_THREAD;
-		else
-			ret = IRQ_HANDLED;
+
+		return IRQ_WAKE_THREAD;
 	}
 
 out:
@@ -1007,9 +1004,6 @@  static irqreturn_t meson_mmc_irq(int irq, void *dev_id)
 		writel(start, host->regs + SD_EMMC_START);
 	}
 
-	if (ret == IRQ_HANDLED)
-		meson_mmc_request_done(host->mmc, cmd->mrq);
-
 	return ret;
 }