Received-SPF: pass (google.com: domain of linux-kernel+bounces-142134-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240410101858.1149134-1-dongliang.cui@unisoc.com>
 <5a67c4f7-4794-45b4-838c-7b739372d3a5@kernel.dk> <5a83c1f7-3ab8-486a-b633-33b44858d290@kernel.org>
In-Reply-To: <5a83c1f7-3ab8-486a-b633-33b44858d290@kernel.org>
From: dongliang cui <cuidongliang390@gmail.com>
Date: Fri, 12 Apr 2024 15:25:25 +0800
Message-ID: <CAPqOJe1FsfA-sx4JXB-UyhUZ4ui3eagFJJ6Z6TfBUpp76ZSw9A@mail.gmail.com>
Subject: Re: [PATCH] block: add max_dispatch to sysfs
To: Damien Le Moal <dlemoal@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>, Dongliang Cui <dongliang.cui@unisoc.com>, ke.wang@unisoc.com, 
	hongyu.jin.cn@gmail.com, niuzhiguo84@gmail.com, linux-block@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 11, 2024 at 1:19=E2=80=AFPM Damien Le Moal <dlemoal@kernel.org>=
 wrote:
>
> On 4/10/24 22:17, Jens Axboe wrote:
> > On 4/10/24 4:18 AM, Dongliang Cui wrote:
> >> The default configuration in the current code is that when the device
> >> is not busy, a single dispatch will attempt to pull 'nr_requests'
> >> requests out of the schedule queue.
> >>
> >> I tried to track the dispatch process:
> >>
> >> COMM            TYPE    SEC_START       IOPRIO       INDEX
> >> fio-17304       R    196798040       0x2005       0
> >> fio-17306       R    197060504       0x2005       1
> >> fio-17307       R    197346904       0x2005       2
> >> fio-17308       R    197609400       0x2005       3
> >> fio-17309       R    197873048       0x2005       4
> >> fio-17310       R    198134936       0x2005       5
> >> ...
> >> fio-17237       R    197122936         0x0       57
> >> fio-17238       R    197384984         0x0       58
> >> <...>-17239     R    197647128         0x0       59
> >> fio-17240       R    197909208         0x0       60
> >> fio-17241       R    198171320         0x0       61
> >> fio-17242       R    198433432         0x0       62
> >> fio-17300       R    195744088       0x2005       0
> >> fio-17301       R    196008504       0x2005       0
> >>
> >> The above data is calculated based on the block event trace, with each
> >> column containing: process name, request type, sector start address,
> >> IO priority.
> >>
> >> The INDEX represents the order in which the requests are extracted fro=
m
> >> the scheduler queue during a single dispatch process.
> >>
> >> Some low-speed devices cannot process these requests at once, and they=
 will
> >> be requeued to hctx->dispatch and wait for the next issuance.
> >>
> >> There will be a problem here, when the IO priority is enabled, if you =
try
> >> to dispatch "nr_request" requests at once, the IO priority will be ign=
ored
> >> from the scheduler queue and all requests will be extracted.
> >>
> >> In this scenario, if a high priority request is inserted into the sche=
duler
> >> queue, it needs to wait for the low priority request in the hctx->disp=
atch
> >> to be processed first.
> >>
> >> --------------------dispatch 1st----------------------
> >> fio-17241       R       198171320         0x0       61
> >> fio-17242       R       198433432         0x0       62
> >> --------------------dispatch 2nd----------------------
> >> fio-17300       R       195744088       0x2005       0
> >>
> >> In certain scenarios, we hope that requests can be processed in order =
of io
> >> priority as much as possible.
> >>
> >> Maybe max_dispatch should not be a fixed value, but can be adjusted
> >> according to device conditions.
> >>
> >> So we give a interface to control the maximum value of single dispatch
> >> so that users can configure it according to devices characteristics.
> >
> > I agree that pulling 'nr_requests' out of the scheduler will kind of
> > defeat the purpose of the scheduler to some extent. But rather than add
> > another knob that nobody knows about or ever will touch (and extra queu=
e
> > variables that just take up space), why not just default to something a
> > bit saner? Eg we could default to 1/8 or 1/4 of the scheduler depth
> > instead.
Current mechanism will try to pulling everything out of the scheduler,
regardless
of the priority of request. Perhaps reducing the queue depth does not solve=
 the
priority disorder scenario. Reducing depth may also weakens the role
of priorify.

>
> Why not default to pulling what can actually be executed, that is, up to =
the
> number of free hw tags / budget ? Anything more than that will be requeue=
d anyway.
The process of pulling the request out of schedule will try to obtain
the hw tags.
If hw tags are obtained, request will continue to be pulled out of schedule=
r.
However, for slow devices, the number of hw tags is generally greater than =
the
number of requests that the device can currently handle, and
unprocessed requests
may not be arranged in dispatch_list in order of priority.
And for budget, I found that the budget is set by the driver. Some
slow devices,
such as emmc, do not register this interface, unable to set budget.

> --
> Damien Le Moal
> Western Digital Research
>