Linux-mmc Archive on lore.kernel.org
 help / color / Atom feed
From: Baolin Wang <baolin.wang7@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Adrian Hunter <adrian.hunter@intel.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	asutoshd@codeaurora.org, Orson Zhai <orsonzhai@gmail.com>,
	Lyra Zhang <zhang.lyra@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Baolin Wang <baolin.wang@linaro.org>,
	linux-mmc <linux-mmc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Paolo Valente <paolo.valente@linaro.org>
Subject: Re: [PATCH v7 1/4] mmc: Add MMC host software queue support
Date: Mon, 9 Dec 2019 17:07:52 +0800
Message-ID: <CADBw62qvkjrhpR7gKpf6NZNHai96Pc8GLHM+5sgNffbWS54WYA@mail.gmail.com> (raw)
In-Reply-To: <CADBw62qW46KyEuj-YOw21sKxLB_uWxxWa_0-0JOXi-6Y48g0hw@mail.gmail.com>

Hi Ulf,

On Fri, Nov 22, 2019 at 6:42 PM Baolin Wang <baolin.wang7@gmail.com> wrote:
>
> Hi Arnd,
>
> On Fri, Nov 22, 2019 at 6:32 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Mon, Nov 18, 2019 at 11:43 AM Baolin Wang <baolin.wang7@gmail.com> wrote:
> > >
> > > From: Baolin Wang <baolin.wang@linaro.org>
> > >
> > > Now the MMC read/write stack will always wait for previous request is
> > > completed by mmc_blk_rw_wait(), before sending a new request to hardware,
> > > or queue a work to complete request, that will bring context switching
> > > overhead, especially for high I/O per second rates, to affect the IO
> > > performance.
> > >
> > > Thus this patch introduces MMC software queue interface based on the
> > > hardware command queue engine's interfaces, which is similar with the
> > > hardware command queue engine's idea, that can remove the context
> > > switching. Moreover we set the default queue depth as 32 for software
> > > queue, which allows more requests to be prepared, merged and inserted
> > > into IO scheduler to improve performance, but we only allow 2 requests
> > > in flight, that is enough to let the irq handler always trigger the
> > > next request without a context switch, as well as avoiding a long latency.
> > >
> > > From the fio testing data in cover letter, we can see the software
> > > queue can improve some performance with 4K block size, increasing
> > > about 16% for random read, increasing about 90% for random write,
> > > though no obvious improvement for sequential read and write.
> > >
> > > Moreover we can expand the software queue interface to support MMC
> > > packed request or packed command in future.
> > >
> > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > > Signed-off-by: Baolin Wang <baolin.wang7@gmail.com>
> >
> > Overall, this looks like enough of a win that I think we should just
> > use the current version for the moment, while still working on all the
> > other improvements.
> >
> > My biggest concern is the naming of "software queue", which is
> > a concept that runs against the idea of doing all the heavy lifting,
> > in particular the queueing in bfq.
> >
> > Then again, it does not /actually/ do much queuing at all, beyond
> > preparing a single request so it can fire it off early. Even with the
> > packed command support added in, there is not really any queuing
> > beyond what it has to do anyway.
>
> Yes. But can not find any better name until now and 'software queue'
> was suggested by Adrian.
>
> >
> > Using the infrastructure that was added for cqe seems like a good
> > compromise, as this already has a way to hand down multiple
> > requests to the hardware and is overall more modern than the
> > existing support.
> >
> > I still think we should do all the other things I mentioned in my
> > earlier reply today, but they can be done as add-ons:
> >
> > - remove all blocking calls from the queue_rq() function:
> >   partition-change, retune, etc should become non-blocking
> >   operations that return busy in the queue_rq function.
> >
> > - get bfq to send down multiple requests all the way into
> >   the device driver, so we don't have to actually queue them
> >   here at all to do packed commands
> >
> > - add packed command support
> >
> > - submit cmds from hardirq context if this is advantageous,
> >   and move everything else in the irq handler into irqthread
> >   context in order to remove all other workqueue and softirq
> >   processing from the request processing path.
> >
> > If we can agree on this as the rough plan for the future,
> > feel free to add my
>
> Yes, I agree with your plan. Thast's what we should do in future.
>
> >
> > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>
> Thanks for your reviewing and good suggestion.
>
> Ulf,
>
> I am not sure if there is any chance to merge this patch set into
> V5.5, I've tested for a long time and did not find any resession.
> Thanks.

Could you apply this patchset if no objection from your side? Or do
you need me to rebase and resend? Thanks.

  reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-18 10:43 [PATCH v7 0/4] Add MMC " Baolin Wang
2019-11-18 10:43 ` [PATCH v7 1/4] mmc: Add MMC host " Baolin Wang
2019-11-22 10:32   ` Arnd Bergmann
2019-11-22 10:42     ` Baolin Wang
2019-12-09  9:07       ` Baolin Wang [this message]
     [not found]         ` <CAMz4kuJ2q_=kEcpz2+GJANdPm5DmwWMLbqBmZHGgtBiEhNFqzw@mail.gmail.com>
     [not found]           ` <CAPDyKFp95H4KVrhiMD9H-C9iZHzEHufNPP95_X7DroYiR+nhHg@mail.gmail.com>
     [not found]             ` <CAMz4kuKRna4s1g3pbw=kCuEnX2voFSh+cQ-mHkrWUoXF9p21XA@mail.gmail.com>
     [not found]               ` <CAPDyKFo3ysxbJr=3fpaEq0rM0qSeCCkLcfA+7mcANQVXYoQ9oA@mail.gmail.com>
     [not found]                 ` <CAMz4kuLQLWYGKTKcycDqWXFPt-aXZvV=geQWbF_aEoh9PE37Yw@mail.gmail.com>
     [not found]                   ` <CAMz4kuKe+Xg=-N2e7V0_GBcddKzfRkt7zRG_j-vjGyFvkXcTMA@mail.gmail.com>
2019-12-19 15:21                     ` Ulf Hansson
2019-12-20  3:50                       ` (Exiting) Baolin Wang
2020-01-21 11:52   ` Ulf Hansson
2020-01-27 10:23     ` (Exiting) Baolin Wang
2019-11-18 10:43 ` [PATCH v7 2/4] mmc: host: sdhci: Add request_done ops for struct sdhci_ops Baolin Wang
2019-11-22 12:13   ` Adrian Hunter
2019-11-18 10:43 ` [PATCH v7 3/4] mmc: host: sdhci: Add a variable to defer to complete requests if needed Baolin Wang
2019-11-22 12:14   ` Adrian Hunter
2019-11-18 10:43 ` [PATCH v7 4/4] mmc: host: sdhci-sprd: Add software queue support Baolin Wang

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CADBw62qvkjrhpR7gKpf6NZNHai96Pc8GLHM+5sgNffbWS54WYA@mail.gmail.com \
    --to=baolin.wang7@gmail.com \
    --cc=adrian.hunter@intel.com \
    --cc=arnd@arndb.de \
    --cc=asutoshd@codeaurora.org \
    --cc=baolin.wang@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=orsonzhai@gmail.com \
    --cc=paolo.valente@linaro.org \
    --cc=ulf.hansson@linaro.org \
    --cc=vincent.guittot@linaro.org \
    --cc=zhang.lyra@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mmc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mmc/0 linux-mmc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mmc linux-mmc/ https://lore.kernel.org/linux-mmc \
		linux-mmc@vger.kernel.org
	public-inbox-index linux-mmc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mmc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git