Re: [PATCHv3 1/1] block: introduce content activity based ioprio

[Zhaoyang Huang]

On Thu, Jan 25, 2024 at 5:32 PM Zhaoyang Huang <huangzhaoyang@gmailcom> wrote:
>
> On Thu, Jan 25, 2024 at 4:26 PM Damien Le Moal <dlemoal@xxxxxxxxxx> wrote:
> >
> > On 1/25/24 16:52, Zhaoyang Huang wrote:
> > > On Thu, Jan 25, 2024 at 3:40 PM Damien Le Moal <dlemoal@xxxxxxxxxx> wrote:
> > >>
> > >> On 1/25/24 16:19, zhaoyang.huang wrote:
> > >>> From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> > >>>
> > >>> Currently, request's ioprio are set via task's schedule priority(when no
> > >>> blkcg configured), which has high priority tasks possess the privilege on
> > >>> both of CPU and IO scheduling.
> > >>> This commit works as a hint of original policy by promoting the request ioprio
> > >>> based on the page/folio's activity. The original idea comes from LRU_GEN
> > >>> which provides more precised folio activity than before. This commit 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 test 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 simply 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 is 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 not 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 similar
> > (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 have.
> 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 process
> > 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==>add_page_to_page_cache(get
> activity information)==>xxx_readpage==>bio_add_page==>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
> >