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

[Adrian Hunter <adrian.hunter@intel.com>]

On 07/08/17 16:41, Ulf Hansson wrote:
> 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!?

HW CMDQ offers 25% - 50% better random multi-threaded I/O.  I see a slight
2% drop in sequential read speed but no change to sequential write.

> 
> **) 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.

There is no advantage to holding up HW CMDQ.

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