Received-SPF: pass (google.com: domain of linux-kernel+bounces-38070-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240124085334.3060748-1-zhaoyang.huang@unisoc.com>
 <ZbDWSUkT/OjHTe0t@infradead.org> <CAGWkznHAi_U5erM0s8vFWwwucRKPoSzimNr9tdsPvs_sApGxgQ@mail.gmail.com>
 <ZbDaq+iI/8ZQWRxF@infradead.org> <CAGWkznGibmzH8WT9CjC9swWD3pLyCcWnG29eoj=EQk=Qd=kHBg@mail.gmail.com>
 <1879ce4b-d8c0-40f4-98ef-88dea0721974@kernel.dk>
In-Reply-To: <1879ce4b-d8c0-40f4-98ef-88dea0721974@kernel.dk>
From: Zhaoyang Huang <huangzhaoyang@gmail.com>
Date: Thu, 25 Jan 2024 15:29:07 +0800
Message-ID: <CAGWkznGGFxazJvhNF-SHFX559sUTkmQfaHpBb-9iMrgdzLaJAw@mail.gmail.com>
Subject: Re: [PATCHv2 1/1] block: introduce content activity based ioprio
To: Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@infradead.org>, "zhaoyang.huang" <zhaoyang.huang@unisoc.com>, 
	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 Wed, Jan 24, 2024 at 11:38=E2=80=AFPM Jens Axboe <axboe@kernel.dk> wrote=
:
>
> On 1/24/24 4:58 AM, Zhaoyang Huang wrote:
> > On Wed, Jan 24, 2024 at 5:38?PM Christoph Hellwig <hch@infradead.org> w=
rote:
> >>
> >> 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
>