linux-kernel - Re: [PATCHv4 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: Tue, 30 Jan 2024 16:37:46 +0800
From: Zhaoyang Huang <huangzhaoyang@...il.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: "zhaoyang.huang" <zhaoyang.huang@...soc.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Jens Axboe <axboe@...nel.dk>, Yu Zhao <yuzhao@...gle.com>, Damien Le Moal <dlemoal@...nel.org>, 
	Niklas Cassel <niklas.cassel@....com>, "Martin K . Petersen" <martin.petersen@...cle.com>, 
	Hannes Reinecke <hare@...e.de>, Linus Walleij <linus.walleij@...aro.org>, linux-mm@...ck.org, 
	linux-block@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, steve.kang@...soc.com
Subject: Re: [PATCHv4 1/1] block: introduce content activity based ioprio

On Fri, Jan 26, 2024 at 10:24 PM Matthew Wilcox <willy@...radead.org> 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 = 0;                            \
> > +             ret = bio_add_page(bio, page, len, offset);             \
> > +             if (ret > 0) {                                          \
> > +                     class = IOPRIO_PRIO_CLASS(bio->bi_ioprio);      \
> > +                     level = IOPRIO_PRIO_LEVEL(bio->bi_ioprio);      \
> > +                     hint = IOPRIO_PRIO_HINT(bio->bi_ioprio);        \
> > +                     activity = IOPRIO_PRIO_ACTIVITY(bio->bi_ioprio);                \
> > +                     activity += (bio->bi_vcnt + 1 <= IOPRIO_NR_ACTIVITY &&          \
> > +                             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.