Received-SPF: pass (google.com: domain of linux-kernel+bounces-44150-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: <20240126120800.3410349-1-zhaoyang.huang@unisoc.com> <ZbPAjGJr7hrOvNOo@casper.infradead.org>
In-Reply-To: <ZbPAjGJr7hrOvNOo@casper.infradead.org>
From: Zhaoyang Huang <huangzhaoyang@gmail.com>
Date: Tue, 30 Jan 2024 16:37:46 +0800
Message-ID: <CAGWkznGSFi9thfuauMtRy6o9YxQV_CiWtFzuxkrTPtvpJrnsiw@mail.gmail.com>
Subject: Re: [PATCHv4 1/1] block: introduce content activity based ioprio
To: Matthew Wilcox <willy@infradead.org>
Cc: "zhaoyang.huang" <zhaoyang.huang@unisoc.com>, Andrew Morton <akpm@linux-foundation.org>, 
	Jens Axboe <axboe@kernel.dk>, Yu Zhao <yuzhao@google.com>, Damien Le Moal <dlemoal@kernel.org>, 
	Niklas Cassel <niklas.cassel@wdc.com>, "Martin K . Petersen" <martin.petersen@oracle.com>, 
	Hannes Reinecke <hare@suse.de>, Linus Walleij <linus.walleij@linaro.org>, linux-mm@kvack.org, 
	linux-block@vger.kernel.org, linux-fsdevel@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 Fri, Jan 26, 2024 at 10:24=E2=80=AFPM Matthew Wilcox <willy@infradead.or=
g> wrote:
>
> On Fri, Jan 26, 2024 at 08:08:00PM +0800, zhaoyang.huang wrote:
> > +#ifdef CONFIG_CONTENT_ACT_BASED_IOPRIO
> > +#define bio_add_page(bio, page, len, offset) \
> > +     ({                                      \
> > +             int class, level, hint, activity;       \
> > +             int ret =3D 0;                            \
> > +             ret =3D bio_add_page(bio, page, len, offset);            =
 \
> > +             if (ret > 0) {                                          \
> > +                     class =3D IOPRIO_PRIO_CLASS(bio->bi_ioprio);     =
 \
> > +                     level =3D IOPRIO_PRIO_LEVEL(bio->bi_ioprio);     =
 \
> > +                     hint =3D IOPRIO_PRIO_HINT(bio->bi_ioprio);       =
 \
> > +                     activity =3D IOPRIO_PRIO_ACTIVITY(bio->bi_ioprio)=
;                \
> > +                     activity +=3D (bio->bi_vcnt + 1 <=3D IOPRIO_NR_AC=
TIVITY &&          \
> > +                             PageWorkingset(&folio->page)) ? 1 : 0;   =
               \
>
> I know you didn't even compile this version.
sorry for forgetting to enable corresponding fs, it will be corrected
in the next patchset. The correct version is compiled and verified by
including act_ioprio.h in the below files in the same android
environment as the previous version.

modified:   fs/erofs/zdata.c
modified:   fs/ext4/page-io.c
modified:   fs/ext4/readpage.c
modified:   fs/f2fs/data.c
modified:   fs/mpage.c

>
> More importantly, conceptually it doesn't work.  All kinds of pages
> get added to bios, and not all of them are file/anon pages.  That
> PageWorkingset bit might well be reused for other purposes.  Only
> the caller knows if this is file/anon memory.  You can't do this here.
>
I noticed the none-file bio's you mentioned such as the one in
xfs_rw_bdev() and fscrypt_zeroout_range(). That's also the reason I
don't define the macro in common fs's header file. It should be up to
fs to decide which bio_add_xxx should be replaced by the activity
based one while keeping others as legacy versions.