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

[Zhaoyang Huang]

On Wed, Jan 24, 2024 at 11:38 PM Jens Axboe <axboe@xxxxxxxxx> wrote:
>
> On 1/24/24 4:58 AM, Zhaoyang Huang wrote:
> > On Wed, Jan 24, 2024 at 5:38?PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> >>
> >> The I/O priority can be explicitly set by the submitter, task and
> >> blkcg arre jut fallbacks.
> > Yes. I would like to suggest if it is possible to have this commit
> > work as a hint for promoting the priority since it has been proved in
> > the verification?
>
> We don't add patches that are wrong just because they provide a
> performance benefit for some cases. Down that path lies tech debt to be
> cleaned up later. Rather, the feature should be done right from the
> start.
>
> >> And as said multiple times now bio_add_page must just treat the page
> >> as a physical address container.  It must never look at MM-internal
> >> flags.
> > The alternative way is to iterate the request;s pages in the scheduler
> > which has been refused by Jens in the previous version. Anyway, we can
> > find a solution on this.
>
> That approach, or the current one, both have the same layering violation
> that Christoph keeps telling you is wrong - you are looking at the page
> itself in the IO path. What has been suggested is that the _issuer_ of
> the IO, the one that actually deals with pages, is the one that should
> be submitting IO at the right priority to begin with.
>
> Your approach tries to hack around the fact that this isn't done, and
> hence is introducing a layering violation where the block layer now
> needs to look at the page and adjust the priority. If the IO was
> submitted with the right priority to begin with, you would not have this
> issue at all.
I have issued out v3 which provide new APIs to have submitter set
bio's ioprio out of bio_add_page
>
> --
> Jens Axboe
>