From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751433AbdJDJjr (ORCPT ); Wed, 4 Oct 2017 05:39:47 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:62581 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751114AbdJDJjn (ORCPT ); Wed, 4 Oct 2017 05:39:43 -0400 X-AuditID: b6c32a37-c75ff70000001076-a2-59d4ac5dae82 From: Bartlomiej Zolnierkiewicz To: Linus Walleij Cc: Adrian Hunter , Ulf Hansson , linux-mmc , linux-block , linux-kernel , Bough Chen , Alex Lemberg , Mateusz Nowak , Yuliy Izrailov , Jaehoon Chung , Dong Aisheng , Das Asutosh , Zhangfei Gao , Sahitya Tummala , Harjani Ritesh , Venu Byravarasu , Shawn Lin , Christoph Hellwig Subject: Re: [PATCH V9 13/15] mmc: block: Add CQE and blk-mq support Date: Wed, 04 Oct 2017 11:39:38 +0200 Message-id: <2384661.GfpEZ6rEzz@amdc3058> User-Agent: KMail/4.13.3 (Linux/3.13.0-96-generic; KDE/4.13.3; x86_64; ; ) In-reply-to: MIME-version: 1.0 Content-transfer-encoding: 7Bit Content-type: text/plain; charset="us-ascii" X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTURzHO3e7d3fW5DZfv4ySBgaZaVHQwR6mWV0iKrKoJVIjLyo5tV21 rP7QHtMemmJqrKiszFrroc731JivRNR8JOWrlxSljkxEM63croH/fc75fT/nfDkcWiTvJF3p 8MgYThOpilBQduLimhXeq4INncrVJtMC3DhgoPC7DxMUrtS+kuDpwmwKX3vzGOHHT+oI/HZS S+LrU3kEruxeiTvKb1G49u81hBMn+ghcPfiaxL0Dz0ncZhyV4IanB/Enc7oImzNfUfhcXj+x xYHtSE0h2DJdn4S9b/pGsL1dJoqteJdAsSnnLRSbbykl2GldtZhNNeoRm2VoJdjRgqV75x+2 2xjCRYTHcRrvzUftwrrPjxLRDz1ONfamkwmoctllJKWBWQe1PVPUZWRHy5lSBC/7qpGwGEdQ lzqK/qd6vhZJrCxnKhDUlEYJoTEEjzouUdYBxfhAepLeJjgynnCl+BdpDYmYjyRcLEmx2Q5M AHwZe2BjMeMOU4ZamyBjVkDh7RYbOzE7wViVRFhZygRCblUZKWQWwkRGv9jKIsYNqqozSYE9 obn+ma02MPUSaM0Zkgi1A+CndogQ2AG+Nxhn9ukZXgztdZuE7WwEJb9BcAsR6CvSZt0NUNPQ NnuBPVjGrpKCK4NkrVyIsPB9OGv2eD/4c6V19unuEPBnfESShpbo5vTWzemtm9P7LhLpkTMX zatDOX5N9FovXqXmYyNDvY5FqQuQ7a95rC9FL1p2mRFDI8UCWUJqh1JOquL4eLUZAS1SOMoa b3Qq5bIQVfxpThN1RBMbwfFmtJgWK1xkzs+7DsmZUFUMd5zjojnN/ylBS10TkL1Giox4e9ii e8p2p0zK50BIfrDhavn7/UHd0nnLBzuzy3qa/U2ue1YVuPh1qWLbb/gEmOVuFkVT4IXw+pOa fQ396r6jJW/5DN/YvPzk3ELfrYk/lN4nmuz1N921w093jDQdEt+GSG466sypmDz/nM+TRSe9 M89aytuCPMV+u7cpxHyYao2HSMOr/gG26uHzZwMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrHIsWRmVeSWpSXmKPExsVy+t9jAd2YNVciDf5MtrI4+WQNm8XNBz/Y LPa2nWC3+Lt5OptF/9WVjBYrVx9lsrjxq43VYsqf5UwWe29pW1zeNYfN4sj/fkaLxh93mSz2 v77AanHnyXpWi4tbPrNbHF8bbvHo0ERmi0NTT7BZNC2/x+Qg7HG5r5fJY+esu+wei/e8ZPK4 c20Pm8fumw1sHr3N79g8Nr7bweTxd9Z+Fo++LasYPaatOc/k8XmTXAB3FJdNSmpOZllqkb5d AlfGrebPTAXLtCpO3pnI2sC4V7GLkZNDQsBE4vbzrexdjFwcQgI7GSUez1/IDOF8ZZT43nSZ DaSKTcBKYmL7KkYQW0RAR6J7209WkCJmgaesElPeHmUGSQgLuEg8+7qEHcRmEVCV+LPmCFgD r4CmxOZ558BsUQEviS372plAbE6BYIk/q6ZCrT7NKPGgtZsFokFQ4sfke2A2s4C8xL79U1kh bC2J9TuPM01g5J+FpGwWkrJZSMoWMDKvYpRMLSjOTc8tNiowzEst1ytOzC0uzUvXS87P3cQI jMVth7X6djDeXxJ/iFGAg1GJh/fGhMuRQqyJZcWVuYcYJTiYlUR4566+EinEm5JYWZValB9f VJqTWnyIUZqDRUmc93besUghgfTEktTs1NSC1CKYLBMHp1QDY5jL+qql7z4u+2+iqWHAPz2v 94ffBxONd9dlxTPqebIZNbPedF1oufpl3eKP9wX+sX3/t+ezZuNqn/3Cme89bu7QMZ06iZlD 6sPlJZVm75be+lFkmXpJP/SFS929Hzv/mG18e3UFd9elCzVsly/7zatKPPGgw+pxia1Rn8ha j7ylux/USjz6ek6JpTgj0VCLuag4EQDK5p7HwQIAAA== X-CMS-MailID: 20171004093941epcas1p1d5277f64b4cc5a78bf185bf9d5b1abfb X-Msg-Generator: CA X-Sender-IP: 182.195.42.142 X-Local-Sender: =?UTF-8?B?QmFydGxvbWllaiBab2xuaWVya2lld2ljehtTUlBPTC1LZXJu?= =?UTF-8?B?ZWwgKFRQKRvsgrzshLHsoITsnpAbU2VuaW9yIFNvZnR3YXJlIEVuZ2luZWVy?= X-Global-Sender: =?UTF-8?B?QmFydGxvbWllaiBab2xuaWVya2lld2ljehtTUlBPTC1LZXJu?= =?UTF-8?B?ZWwgKFRQKRtTYW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBF?= =?UTF-8?B?bmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 101P X-CMS-RootMailID: 20171004093941epcas1p1d5277f64b4cc5a78bf185bf9d5b1abfb X-RootMTR: 20171004093941epcas1p1d5277f64b4cc5a78bf185bf9d5b1abfb References: <1506083824-4024-1-git-send-email-adrian.hunter@intel.com> <1506083824-4024-14-git-send-email-adrian.hunter@intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Wednesday, October 04, 2017 09:39:45 AM Linus Walleij wrote: > On Fri, Sep 22, 2017 at 2:37 PM, Adrian Hunter 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 > > I am trying to wrap my head around this large patch. The size makes it hard > but I am doing my best. I also think that this patch should be split on two patches. The 1st one introducing blk-mq and the 2nd one adding CQE support. [ I don't agree that they make more sense together, on the contrary, it is very difficult to properly analyze blk-mq changes on their own while there are mixed with CQE related ones. ] > Some overarching questions: > > - Is the CQE only available on the MQ path (i.e. if you enabled MQ) or > on both paths? > > I think it is reasonable that if we introduce a new feature like this > it will only > be available for the new block path. This reflects how the block maintainers > e.g. only allow new scheduling policies to be merged on the MQ path. > The old block layer is legacy and should not be extended with new cool > features that can then be regarded as "regressions" if they don't work > properly with MQ. Better to only implement them for MQ then. > > - Performance before/after path on MQ? > > I tested this very extensively when working with my (now dormant) MQ > patch set. Better/equal/worse? > (https://marc.info/?l=linux-mmc&m=148665788227015&w=2) > > The reason my patch set contained refactorings of async post-processing, > removed the waitqueues and the context info, up to the point where I > can issue requests in parallel, i.e. complete requests from the ->done() > callback on the host and immediately let the core issue the next one, > was due to performance issues. > > It's these patches from the old patch set: > > mmc: core: move some code in mmc_start_areq() > mmc: core: refactor asynchronous request finalization > mmc: core: refactor mmc_request_done() > mmc: core: move the asynchronous post-processing > mmc: core: add a kthread for completing requests > mmc: core: replace waitqueue with worker > mmc: core: do away with is_done_rcv > mmc: core: do away with is_new_req > mmc: core: kill off the context info > mmc: queue: simplify queue logic > mmc: block: shuffle retry and error handling > mmc: queue: stop flushing the pipeline with NULL > mmc: queue: issue struct mmc_queue_req items > mmc: queue: get/put struct mmc_queue_req > mmc: queue: issue requests in massive parallel > > I.e. I made 15 patches just to make sure the new block layer did not > regress performance. The MQ-switch patch was just the final step of > these 16 patches. > > Most energy went into that and I think it will be necessary still to work > with MQ in the long haul. > > I am worried that this could add a second MQ execution path that > performs worse than the legacy block path for the above reason, i.e. > it doesn't really take advantage of the MQ speedups by marshalling > the requests and not doing away with the waitqueues and not > completing the requests out-of-order, so we get stuck with a lump of BTW The out-of-order approach taken by the above patchset seems racy (at least when it comes to error handling), the details were in my review mails which have not been answered yet: https://www.spinics.net/lists/linux-mmc/msg42751.html Also the patchset itself has never worked on my setup: https://www.spinics.net/lists/linux-mmc/msg42759.html > MQ code that doesn't perform and therefore we cannot switch seamlessly > to MQ. I think that switching seamlessly to blk-mq in short/medium-term is not possible (SCSI tried and failed to do so). The changes to the old path are very complex and besides affecting performance they also affect error recovery handling (which needs to be tested properly before the switch). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics