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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Thu, 25 Jan 2024 15:29:07 +0800
From: Zhaoyang Huang <huangzhaoyang@...il.com>
To: Jens Axboe <axboe@...nel.dk>
Cc: Christoph Hellwig <hch@...radead.org>, "zhaoyang.huang" <zhaoyang.huang@...soc.com>, 
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org, 
	steve.kang@...soc.com
Subject: Re: [PATCHv2 1/1] block: introduce content activity based ioprio

On Wed, Jan 24, 2024 at 11:38 PM Jens Axboe <axboe@...nel.dk> wrote:
>
> On 1/24/24 4:58 AM, Zhaoyang Huang wrote:
> > On Wed, Jan 24, 2024 at 5:38?PM Christoph Hellwig <hch@...radead.org> 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
>