Re: [PATCH V4 00/11] mmc: Add Command Queue support

Ulf Hansson
2017-08-07T13:41:38Z

On 21 July 2017 at 11:49, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hi
>
> Here is V4 of the hardware command queue patches without the software
> command queue patches.

Adrian, again apologize for the delay.

I am reviewing the series now and my intent is to provide comments on
each change separately during the week.

However, a couple of overall thoughts:

*) Could you please post some fresh performance measurements, such we
can get some real proof on why CMDQ is worth to be merged? If not
increased throughput, perhaps we can show some decreased I/O requests
latency!?

**) I have spoken to Linus W offlist - and he is still working on the
blkmq port. Although we first need to continue with the
re-factorization of the mmc block/core code, to minimize the bad
impact of our big mmc claim host lock. We should expect a
re-submission of his series quite soonish. However, it's also likely
that we need yet another round of re-factoring, before we can complete
the port to blkmq.

***) The reason for bringing up **), is of course because I think
deploying CMDQ support would be better done on top of the blkmq
interface as it's better suited for these kind of device types. My
goal is to reach a better maintenance situation, using more modern mmc
block code. We have spoken about this before, however of course I also
don't want to delay the CMDQ series. Anyway, let's move forward and
see what path we end up taking.

>
>
> Changes since V3:
> Adjusted ...blk_end_request...() for new block status codes
> Fixed CQHCI transaction descriptor for "no DCMD" case
>
> Changes since V2:
> Dropped patches that have been applied.
> Re-based
> Added "mmc: sdhci-pci: Add CQHCI support for Intel GLK"
>
> Changes since V1:
>
> "Share mmc request array between partitions" is dependent
> on changes in "Introduce queue semantics", so added that
> and block fixes:
>
> Added "Fix is_waiting_last_req set incorrectly"
> Added "Fix cmd error reset failure path"
> Added "Use local var for mqrq_cur"
> Added "Introduce queue semantics"
>
> Changes since RFC:
>
> Re-based on next.
> Added comment about command queue priority.
> Added some acks and reviews.
>
>
> Adrian Hunter (10):
> mmc: core: Add mmc_retune_hold_now()
> mmc: core: Add members to mmc_request and mmc_data for CQE's
> mmc: host: Add CQE interface
> mmc: core: Turn off CQE before sending commands
> mmc: core: Add support for handling CQE requests
> mmc: mmc: Enable Command Queuing
> mmc: mmc: Enable CQE's
> mmc: block: Prepare CQE data
> mmc: block: Add CQE support
> mmc: sdhci-pci: Add CQHCI support for Intel GLK
>
> Venkat Gopalakrishnan (1):
> mmc: cqhci: support for command queue enabled host
>
> drivers/mmc/core/block.c | 240 +++++++-
> drivers/mmc/core/block.h | 7 +
> drivers/mmc/core/bus.c | 7 +
> drivers/mmc/core/core.c | 156 ++++-
> drivers/mmc/core/host.c | 6 +
> drivers/mmc/core/host.h | 1 +
> drivers/mmc/core/mmc.c | 30 +-
> drivers/mmc/core/queue.c | 273 ++++++++-
> drivers/mmc/core/queue.h | 42 +-
> drivers/mmc/host/Kconfig | 14 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/cqhci.c | 1146 +++++++++++++++++++++++++++++++++++++
> drivers/mmc/host/cqhci.h | 240 ++++++++
> drivers/mmc/host/sdhci-pci-core.c | 153 ++++-
> include/linux/mmc/core.h | 18 +-
> include/linux/mmc/host.h | 24 +
> include/trace/events/mmc.h | 36 +-
> 17 files changed, 2354 insertions(+), 40 deletions(-)
> create mode 100644 drivers/mmc/host/cqhci.c
> create mode 100644 drivers/mmc/host/cqhci.h
>
>
> Regards
> Adrian

Kind regards
Uffe