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 >
