Received-SPF: pass (google.com: domain of linux-kernel+bounces-29905-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240117092348.2873928-1-zhaoyang.huang@unisoc.com> <5ff36535-7cd3-4e8f-a477-9d2a98d18dd9@acm.org>
In-Reply-To: <5ff36535-7cd3-4e8f-a477-9d2a98d18dd9@acm.org>
From: Zhaoyang Huang <huangzhaoyang@gmail.com>
Date: Thu, 18 Jan 2024 16:40:37 +0800
Message-ID: <CAGWkznGc67yY9xCHJFLGvJZBu2oXOgOgHKZ_OvbBrv5mqRMywg@mail.gmail.com>
Subject: Re: [RFC PATCH 1/1] block: introduce activity based ioprio
To: Bart Van Assche <bvanassche@acm.org>
Cc: "zhaoyang.huang" <zhaoyang.huang@unisoc.com>, Jens Axboe <axboe@kernel.dk>, 
	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 Thu, Jan 18, 2024 at 1:29=E2=80=AFAM Bart Van Assche <bvanassche@acm.org=
> wrote:
>
> On 1/17/24 01:23, zhaoyang.huang wrote:
> > +static enum dd_prio dd_req_ioprio(struct request *rq)
> > +{
> > +     enum dd_prio prio;
> > +     const u8 ioprio_class =3D dd_rq_ioclass(rq);
> > +#ifdef CONFIG_ACTIVITY_BASED_IOPRIO
> > +     struct bio *bio;
> > +     struct bio_vec bv;
> > +     struct bvec_iter iter;
> > +     struct page *page;
> > +     int gen =3D 0;
> > +     int cnt =3D 0;
> > +
> > +     if (req_op(rq) =3D=3D REQ_OP_READ) {
> > +             __rq_for_each_bio(bio, rq) {
> > +                     bio_for_each_bvec(bv, bio, iter) {
> > +                             page =3D bv.bv_page;
> > +                             gen +=3D PageWorkingset(page) ? 1 : 0;
> > +                             cnt++;
> > +                     }
> > +             }
> > +             prio =3D (gen >=3D cnt / 2) ? ioprio_class_to_prio[IOPRIO=
_CLASS_RT] :
> > +                     ioprio_class_to_prio[ioprio_class];
> > +     } else
> > +             prio =3D ioprio_class_to_prio[ioprio_class];
> > +#else
> > +     prio =3D ioprio_class_to_prio[ioprio_class];
> > +#endif
> > +     return prio;
> > +}
>
> I don't like it that code is introduced in the mq-deadline scheduler
> that accesses page cache information. Isn't that a layering violation?
ok. I will try to update a new version to implement these in block layer

> Additionally, this approach only works for buffered I/O and not for
> direct I/O. Shouldn't the I/O submitter set the I/O priority instead of
> deciding the I/O priority in the mq-deadline scheduler?
That's just the purpose of this commit, that is, introducing content
activity based ioprio OR the one that submitter decided
>
> Bart.