Received-SPF: pass (google.com: domain of linux-kernel+bounces-38288-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240125071901.3223188-1-zhaoyang.huang@unisoc.com>
 <6b2d5694-f802-43a4-a0fd-1c8e34f8e69a@kernel.org> <CAGWkznHK5UPajY2PG24Jm7+A0c9q+tyQzrPdd=n3tp0dgX+T0w@mail.gmail.com>
 <95082224-a61d-4f4b-bc96-1beea8aa93a9@kernel.org> <CAGWkznGe+37K7_E34G_MZQXNxoLuM3E0mY=BZCGpyiq+TB_PoA@mail.gmail.com>
In-Reply-To: <CAGWkznGe+37K7_E34G_MZQXNxoLuM3E0mY=BZCGpyiq+TB_PoA@mail.gmail.com>
From: Zhaoyang Huang <huangzhaoyang@gmail.com>
Date: Thu, 25 Jan 2024 17:37:49 +0800
Message-ID: <CAGWkznGJejDMZsryPpS-11mb50wMhhQe-=sNoV4_iO5ogVLtLQ@mail.gmail.com>
Subject: Re: [PATCHv3 1/1] block: introduce content activity based ioprio
To: Damien Le Moal <dlemoal@kernel.org>
Cc: "zhaoyang.huang" <zhaoyang.huang@unisoc.com>, Andrew Morton <akpm@linux-foundation.org>, 
	Jens Axboe <axboe@kernel.dk>, Yu Zhao <yuzhao@google.com>, 
	Niklas Cassel <niklas.cassel@wdc.com>, "Martin K . Petersen" <martin.petersen@oracle.com>, 
	Hannes Reinecke <hare@suse.de>, Linus Walleij <linus.walleij@linaro.org>, linux-mm@kvack.org, 
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, 
	steve.kang@unisoc.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Jan 25, 2024 at 5:32=E2=80=AFPM Zhaoyang Huang <huangzhaoyang@gmail=
com> wrote:
>
> On Thu, Jan 25, 2024 at 4:26=E2=80=AFPM Damien Le Moal <dlemoal@kernel.or=
g> wrote:
> >
> > On 1/25/24 16:52, Zhaoyang Huang wrote:
> > > On Thu, Jan 25, 2024 at 3:40=E2=80=AFPM Damien Le Moal <dlemoal@kerne=
l.org> wrote:
> > >>
> > >> On 1/25/24 16:19, zhaoyang.huang wrote:
> > >>> From: Zhaoyang Huang <zhaoyang.huang@unisoc.com>
> > >>>
> > >>> Currently, request's ioprio are set via task's schedule priority(wh=
en no
> > >>> blkcg configured), which has high priority tasks possess the privil=
ege on
> > >>> both of CPU and IO scheduling.
> > >>> This commit works as a hint of original policy by promoting the req=
uest ioprio
> > >>> based on the page/folio's activity. The original idea comes from LR=
U_GEN
> > >>> which provides more precised folio activity than before. This commi=
t try
> > >>> to adjust the request's ioprio when certain part of its folios are =
hot,
> > >>> which indicate that this request carry important contents and need =
be
> > >>> scheduled ealier.
> > >>>
> > >>> This commit is verified on a v6.6 6GB RAM android14 system via 4 te=
st cases
> > >>> by changing the bio_add_page/folio API in ext4 and f2fs.
> > >>
> > >> And as mentioned already by Chrisoph and Jens, why don't you just si=
mply set
> > >> bio->bi_ioprio to the value you want before calling submit_bio() in =
these file
> > >> systems ? Why all the hacking of the priority code for that ? That i=
s not
> > >> justified at all.
> > >>
> > >> Furthermore, the activity things reduces the ioprio hint bits to the=
 bare
> > >> minimum 3 bits necessary for command duration limits. Not great. But=
 if you
> > >> simply set the prio class based on your activity algorithm, you do n=
ot need to
> > >> change all that.
> > > That is because bio->io_prio changes during bio grows with adding
> > > different activity pages in. I have to wrap these into an API which
> > > has both of fs and block be transparent to the process.
> >
> > Pages are not added to BIOs on the fly. The FS does bio_add_page() or s=
imilar
> > (it can be a get user pages for direct IOs) and then calls bio_submit()=
 Between
> > these 2, you can set your IO priority according to how many pages you h=
ave.
> Please correct me if I am wrong. So you suggest iterating the
> request->bios->bvecs(pages) before final submit_bio? Is it too costly
> and introduces too many modifications on each fs.
> >
> > You can even likely do all of this based on the iocb (and use iocb->ki_=
ioprio to
> > set the prio), so before one starts allocating and setting up BIOs to p=
rocess
> > the user IO.
> Actually, the activity information comes from page's history (recorded
> at page cache's slot) instead of user space in step(1) and can be
> associate with bio in step(2) or iterate the bio in step(3)
>
> page fault        \
> (1)
>                               (2)                       (3)
>                           allocate_pages=3D=3D>add_page_to_page_cache(get
> activity information)=3D=3D>xxx_readpage=3D=3D>bio_add_page=3D=3D>submit_=
bio
> vfs read/write  /
The chart goes wrong again:(           change it to vertical mode
Actually, the activity information comes from page's history (recorded
at page cache's slot) instead of user space in step(1) and can be
associate with bio in step(2) or iterate the bio in step(3)
page fault(or vfs)(1)
|
alloc_pages
|
add_page_to_pagecache(get the page's activity information)
|
fs_readpage
|
bio_add_page(2)
|
submit_bio(3)

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