[PATCH] mmc: dw_mmc: enable MMC_CAP_DONE_COMPLETE

[PATCH] mmc: dw_mmc: enable MMC_CAP_DONE_COMPLETE

[Shawn Lin]

All requests issued by dw_mmc are completed from dw_mci_work_func(),
which runs from the system_bh_wq workqueue. It is therefore preemptible
context and host->lock is dropped before the completion, see
mmc_request_done() in dw_mci_request_end().

The dw_mci_interrupt and the dw_mci_watchdog_fn never call mmc_request_done()
directly. So every request is completed from a sleepable context with no
spinlock held.

This is exactly the contract required by MMC_CAP_DONE_COMPLETE, which was
introduced for hosts that can complete requests within mmc_request_done()
without deferring to the core's complete_work.

Without the flag, the core records the finished request in
mq->complete_req and queues mq->complete_work, i.e. a second workqueue
dispatch plus an extra context switch per request. With the flag,
mmc_blk_mq_req_done() completes the request directly via
mmc_blk_mq_post_req() -> blk_mq_complete_request(), and a following
request completes the previous one inline from mmc_blk_rw_wait().

Measured on an RK3576 SD card (dw_mmc) with:

  perf record -a -g -F 999 -- sleep 30
  perf report --stdio -g graph,0.5,caller

fio: 4k randread, ioengine=libaio

Without the flag, the completion hot path is:

  mmc_request_done -> mmc_blk_mq_req_done -> queue_work_on      (~4.3%)
  ... -> mmc_blk_mq_complete_work            (kworker/5:1H-mm, ~18.4%)
       -> mmc_blk_mq_complete_prev_req -> mmc_blk_mq_post_req

with the flag the workqueue round trip disappears completely: there is no
queue_work_on and no mmc_blk_mq_complete_work in the profile anymore, and
the previous request is completed inline:

  mmc_blk_mq_issue_rq -> mmc_blk_mq_post_req -> mmc_blk_mq_complete_rq
                                                  -> __blk_mq_end_request

This can remove a redundant workqueue dispatch and context switch per completed
request.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/dw_mmc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ff0b25d..bec777b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2945,6 +2945,8 @@ static int dw_mci_init_host_caps(struct dw_mci *host)
 	if (mmc->caps & MMC_CAP_SDIO_IRQ)
 		mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
 
+	mmc->caps |= MMC_CAP_DONE_COMPLETE;
+
 	return 0;
 }
 
-- 
2.7.4

Re: [PATCH] mmc: dw_mmc: enable MMC_CAP_DONE_COMPLETE

[Ulf Hansson]

On Tue, Oct 13, 2026 at 9:24 AM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> All requests issued by dw_mmc are completed from dw_mci_work_func(),
> which runs from the system_bh_wq workqueue. It is therefore preemptible
> context and host->lock is dropped before the completion, see
> mmc_request_done() in dw_mci_request_end().
>
> The dw_mci_interrupt and the dw_mci_watchdog_fn never call mmc_request_done()
> directly. So every request is completed from a sleepable context with no
> spinlock held.
>
> This is exactly the contract required by MMC_CAP_DONE_COMPLETE, which was
> introduced for hosts that can complete requests within mmc_request_done()
> without deferring to the core's complete_work.
>
> Without the flag, the core records the finished request in
> mq->complete_req and queues mq->complete_work, i.e. a second workqueue
> dispatch plus an extra context switch per request. With the flag,
> mmc_blk_mq_req_done() completes the request directly via
> mmc_blk_mq_post_req() -> blk_mq_complete_request(), and a following
> request completes the previous one inline from mmc_blk_rw_wait().
>
> Measured on an RK3576 SD card (dw_mmc) with:
>
>   perf record -a -g -F 999 -- sleep 30
>   perf report --stdio -g graph,0.5,caller
>
> fio: 4k randread, ioengine=libaio
>
> Without the flag, the completion hot path is:
>
>   mmc_request_done -> mmc_blk_mq_req_done -> queue_work_on      (~4.3%)
>   ... -> mmc_blk_mq_complete_work            (kworker/5:1H-mm, ~18.4%)
>        -> mmc_blk_mq_complete_prev_req -> mmc_blk_mq_post_req
>
> with the flag the workqueue round trip disappears completely: there is no
> queue_work_on and no mmc_blk_mq_complete_work in the profile anymore, and
> the previous request is completed inline:
>
>   mmc_blk_mq_issue_rq -> mmc_blk_mq_post_req -> mmc_blk_mq_complete_rq
>                                                   -> __blk_mq_end_request
>
> This can remove a redundant workqueue dispatch and context switch per completed
> request.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
>
>  drivers/mmc/host/dw_mmc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ff0b25d..bec777b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2945,6 +2945,8 @@ static int dw_mci_init_host_caps(struct dw_mci *host)
>         if (mmc->caps & MMC_CAP_SDIO_IRQ)
>                 mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
>
> +       mmc->caps |= MMC_CAP_DONE_COMPLETE;
> +
>         return 0;
>  }
>
> --
> 2.7.4
>

Re: [PATCH] mmc: dw_mmc: enable MMC_CAP_DONE_COMPLETE

[Marek Szyprowski]

On 13.10.2026 09:24, Shawn Lin wrote:
> All requests issued by dw_mmc are completed from dw_mci_work_func(),
> which runs from the system_bh_wq workqueue. It is therefore preemptible
> context and host->lock is dropped before the completion, see
> mmc_request_done() in dw_mci_request_end().
>
> The dw_mci_interrupt and the dw_mci_watchdog_fn never call mmc_request_done()
> directly. So every request is completed from a sleepable context with no
> spinlock held.
>
> This is exactly the contract required by MMC_CAP_DONE_COMPLETE, which was
> introduced for hosts that can complete requests within mmc_request_done()
> without deferring to the core's complete_work.
>
> Without the flag, the core records the finished request in
> mq->complete_req and queues mq->complete_work, i.e. a second workqueue
> dispatch plus an extra context switch per request. With the flag,
> mmc_blk_mq_req_done() completes the request directly via
> mmc_blk_mq_post_req() -> blk_mq_complete_request(), and a following
> request completes the previous one inline from mmc_blk_rw_wait().
>
> Measured on an RK3576 SD card (dw_mmc) with:
>
>   perf record -a -g -F 999 -- sleep 30
>   perf report --stdio -g graph,0.5,caller
>
> fio: 4k randread, ioengine=libaio
>
> Without the flag, the completion hot path is:
>
>   mmc_request_done -> mmc_blk_mq_req_done -> queue_work_on      (~4.3%)
>   ... -> mmc_blk_mq_complete_work            (kworker/5:1H-mm, ~18.4%)
>        -> mmc_blk_mq_complete_prev_req -> mmc_blk_mq_post_req
>
> with the flag the workqueue round trip disappears completely: there is no
> queue_work_on and no mmc_blk_mq_complete_work in the profile anymore, and
> the previous request is completed inline:
>
>   mmc_blk_mq_issue_rq -> mmc_blk_mq_post_req -> mmc_blk_mq_complete_rq
>                                                   -> __blk_mq_end_request
>
> This can remove a redundant workqueue dispatch and context switch per completed
> request.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>


This patch landed yesterday in linux-next as commit 80636b762eec ("mmc: dw_mmc:
enable MMC_CAP_DONE_COMPLETE"). In my tests I found that it causes serious
problems. I found that it is responsible for corrupting root filesystem and/or
journal.
Reverting this on top of linux-next fixes the issue. Unfortunately I didn't find
what mmc access pattern causes the corruption, but running my tests and rebooting
boards 2 or 3 times usually ended in breaking it.


There must some corner case missed in the analysis behind this patch.



> ---
>
>  drivers/mmc/host/dw_mmc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ff0b25d..bec777b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2945,6 +2945,8 @@ static int dw_mci_init_host_caps(struct dw_mci *host)
>  	if (mmc->caps & MMC_CAP_SDIO_IRQ)
>  		mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
>  
> +	mmc->caps |= MMC_CAP_DONE_COMPLETE;
> +
>  	return 0;
>  }
>  

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] mmc: dw_mmc: enable MMC_CAP_DONE_COMPLETE

[Shawn Lin]

Hi Marek,

在 2026/11/11 星期三 3:28, Marek Szyprowski 写道:
> On 13.10.2026 09:24, Shawn Lin wrote:
>> All requests issued by dw_mmc are completed from dw_mci_work_func(),
>> which runs from the system_bh_wq workqueue. It is therefore preemptible
>> context and host->lock is dropped before the completion, see
>> mmc_request_done() in dw_mci_request_end().
>>
>> The dw_mci_interrupt and the dw_mci_watchdog_fn never call mmc_request_done()
>> directly. So every request is completed from a sleepable context with no
>> spinlock held.
>>
>> This is exactly the contract required by MMC_CAP_DONE_COMPLETE, which was
>> introduced for hosts that can complete requests within mmc_request_done()
>> without deferring to the core's complete_work.
>>
>> Without the flag, the core records the finished request in
>> mq->complete_req and queues mq->complete_work, i.e. a second workqueue
>> dispatch plus an extra context switch per request. With the flag,
>> mmc_blk_mq_req_done() completes the request directly via
>> mmc_blk_mq_post_req() -> blk_mq_complete_request(), and a following
>> request completes the previous one inline from mmc_blk_rw_wait().
>>
>> Measured on an RK3576 SD card (dw_mmc) with:
>>
>>    perf record -a -g -F 999 -- sleep 30
>>    perf report --stdio -g graph,0.5,caller
>>
>> fio: 4k randread, ioengine=libaio
>>
>> Without the flag, the completion hot path is:
>>
>>    mmc_request_done -> mmc_blk_mq_req_done -> queue_work_on      (~4.3%)
>>    ... -> mmc_blk_mq_complete_work            (kworker/5:1H-mm, ~18.4%)
>>         -> mmc_blk_mq_complete_prev_req -> mmc_blk_mq_post_req
>>
>> with the flag the workqueue round trip disappears completely: there is no
>> queue_work_on and no mmc_blk_mq_complete_work in the profile anymore, and
>> the previous request is completed inline:
>>
>>    mmc_blk_mq_issue_rq -> mmc_blk_mq_post_req -> mmc_blk_mq_complete_rq
>>                                                    -> __blk_mq_end_request
>>
>> This can remove a redundant workqueue dispatch and context switch per completed
>> request.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> 
> This patch landed yesterday in linux-next as commit 80636b762eec ("mmc: dw_mmc:
> enable MMC_CAP_DONE_COMPLETE"). In my tests I found that it causes serious
> problems. I found that it is responsible for corrupting root filesystem and/or
> journal.
> Reverting this on top of linux-next fixes the issue. Unfortunately I didn't find
> what mmc access pattern causes the corruption, but running my tests and rebooting
> boards 2 or 3 times usually ended in breaking it.
> 
> 
> There must some corner case missed in the analysis behind this patch.
> 

Thansk for testing.

I have tested this patch in my local product environment for several
days and did not trigger the issue. Of course, I have also been keeping
an eye on whether the Linux-next branch can reproduce the problem on
other platforms, because frankly speaking, this is the first controller
that is about to enable this mmc block multi-queue internal mechanism,
meaning this mechanism has not undergone rigorous large-scale deployment
testing. Could you please help test the following patch to see if it
resolves your issue?

--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2248,6 +2248,7 @@ static void mmc_blk_mq_req_done(struct mmc_request 
*mrq)

         /* Take the recovery path for errors or urgent background 
operations */
         if (mmc_blk_rq_error(&mqrq->brq) ||
+           mmc_blk_card_busy(mq->card, req) ||
             mmc_blk_urgent_bkops_needed(mq, mqrq)) {
                 spin_lock_irqsave(&mq->lock, flags);
                 mq->recovery_needed = true;


> 
> 
>> ---
>>
>>   drivers/mmc/host/dw_mmc.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index ff0b25d..bec777b 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -2945,6 +2945,8 @@ static int dw_mci_init_host_caps(struct dw_mci *host)
>>   	if (mmc->caps & MMC_CAP_SDIO_IRQ)
>>   		mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
>>   
>> +	mmc->caps |= MMC_CAP_DONE_COMPLETE;
>> +
>>   	return 0;
>>   }
>>   
> 
> Best regards