Linus, Adrian,

On 28 November 2017 at 10:42, Linus Walleij [off-list ref] wrote:

On Tue, Nov 21, 2017 at 2:42 PM, Adrian Hunter [off-list ref] wrote:

quoted

Here is V14 of the hardware command queue patches without the software
command queue patches, now using blk-mq and now with blk-mq support for
non-CQE I/O.

V14 includes a number of fixes to existing code, changes to default to
blk-mq, and adds patches to remove legacy code.

I have looked over the code, I was unable to find a good mergebase to apply
it on (I guess it is based on linux-next at some date in the past) so mostly
I just looked at it overall, and I can solidly say that this patch series:

Acked-by: Linus Walleij <redacted>

Great, thanks!

I gave some more explicit review on some initial patches that I think
should go in as fixes.

Thanks, and already taken care of.

I do not expect it to perform any less than the previous iteration on my
systems where it was already performing well and Bartlomiej also has
confirmed that the patch set works for him.

Ulf: I suggest this be applied (+/- some rebasing) early for v4.15.

Yes, I am up for that!

I have now also completed my review of the series and in the end, most
of my comments turned out to be of minor issues, hopefully easily
addressed.

I am positively convinced that we can make things work on top of this.

quoted

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.

Fully acceptable I think.

quoted

Non-CQE blk-mq showed a 3% decrease in sequential read performance.  This
seemed to be coming from the inferior latency of running work items compared
with a dedicated thread.  Hacking blk-mq workqueue to be unbound reduced the
performance degradation from 3% to 1%.

Also acceptable I think.

quoted

While we should look at changing blk-mq to give better workqueue performance,
a bigger gain is likely to be made by adding a new host API to enable the
next already-prepared request to be issued directly from within ->done()
callback of the current request.

I assume that is taken care of by adding the new host cap
(MMC_CAP_DIRECT_COMPLETE).

So, then it's just a matter of adopting all host drivers, and when we
are done with that, we can remove that cap. :-)

I agree.

Yours,
Linus Walleij

I am awaiting a re-based v15 version - and eager to apply it! :-)

Thanks and kind regards
Uffe