Re: [PATCH V8 12/14] mmc: block: Add CQE and blk-mq support

From: Ulf Hansson
Date: Thu Sep 21 2017 - 05:59:14 EST


On 13 September 2017 at 13:40, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> Add CQE support to the block driver, including:
> - optionally using DCMD for flush requests
> - "manually" issuing discard requests
> - issuing read / write requests to the CQE
> - supporting block-layer timeouts
> - handling recovery
> - supporting re-tuning
>
> CQE offers 25% - 50% better random multi-threaded I/O. There is a slight
> (e.g. 2%) drop in sequential read speed but no observable change to sequential
> write.
>
> CQE automatically sends the commands to complete requests. However it only
> supports reads / writes and so-called "direct commands" (DCMD). Furthermore
> DCMD is limited to one command at a time, but discards require 3 commands.
> That makes issuing discards through CQE very awkward, but some CQE's don't
> support DCMD anyway. So for discards, the existing non-CQE approach is
> taken, where the mmc core code issues the 3 commands one at a time i.e.
> mmc_erase(). Where DCMD is used, is for issuing flushes.
>
> For host controllers without CQE support, blk-mq support is extended to
> synchronous reads/writes or, if the host supports CAP_WAIT_WHILE_BUSY,
> asynchonous reads/writes. The advantage of asynchronous reads/writes is
> that it allows the preparation of the next request while the current
> request is in progress.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
> drivers/mmc/core/block.c | 734 ++++++++++++++++++++++++++++++++++++++++++++++-
> drivers/mmc/core/block.h | 8 +
> drivers/mmc/core/queue.c | 428 +++++++++++++++++++++++++--
> drivers/mmc/core/queue.h | 54 +++-
> 4 files changed, 1192 insertions(+), 32 deletions(-)

Adrian, this is just too hard for me to review. Please, can you split this up?

At least enabling mq support should be done in a separate and first
step, then you could add the CQE bits on top.

I think that is important, because we want to make sure the mq
deployment is done correctly first. Otherwise it becomes impossible to
narrow down problems, because those could then be either CQE related
or mq related.

[....]

Kind regards
Uffe