Re: [PATCH] mmc: dw_mmc: enable MMC_CAP_DONE_COMPLETE

[Ulf Hansson]

On Tue, Nov 10, 2026 at 8:28 PM Marek Szyprowski
<m.szyprowski@xxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxx>
>
>
> 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.

Okay, that's not good! I have reverted the patch on my next branch for
now and added your reported-by tag for it.

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

Yep, let's see if we can make it work. I assume the benefits should be worth it.

>
>
>
> > ---
> >
> >  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
>

Kind regards
Uffe