linux-kernel - Re: [RFC PATCH 2/2] mm: introduce budgt control in readahead

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: Wed, 15 May 2024 16:17:32 +0800
From: Zhaoyang Huang <huangzhaoyang@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: "zhaoyang.huang" <zhaoyang.huang@...soc.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Matthew Wilcox <willy@...radead.org>, Jens Axboe <axboe@...nel.dk>, 
	Josef Bacik <josef@...icpanda.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>, linux-mm@...ck.org, 
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org, 
	cgroups@...r.kernel.org, steve.kang@...soc.com
Subject: Re: [RFC PATCH 2/2] mm: introduce budgt control in readahead

On Wed, May 15, 2024 at 3:40 PM Tejun Heo <tj@...nel.org> wrote:
>
> Hello,
>
> On Wed, May 15, 2024 at 09:23:50AM +0800, zhaoyang.huang wrote:
> > +static unsigned long get_next_ra_size(struct readahead_control *ractl,
> >                                     unsigned long max)
> >  {
> > +     unsigned long cur = ractl->ra->size;
> > +     struct inode *inode = ractl->mapping->host;
> > +     unsigned long budgt = inode->i_sb->s_bdev ?
> > +                     blk_throttle_budgt(inode->i_sb->s_bdev) : 0;
>
> Technical correctness aside, I'm not convinced it's generally a good idea to
> bubble up one specific IO control mechanism's detail all the way upto RA
> layer. Besides what's the gain here? For continuous IO stream, whether some
> RA bios are oversized or not shouldn't matter, no? Doesn't this just affect
> the accuracy of the last RA IO of a finite read stream?
Thanks for feedback. If I understand right, the oversized RA bios of a
finite read will fail by being queued to tg's queue which should be
deemed as introducing a drop of IOPS.

submit_bio
    blk_throtl_bio
        if(!tg_may_dispatch) //failed, queue the bio to tg's queue

What we get here is a more precise BW of the throttled blkcg like
below, from which we can find the result of 'after' could exactly meet
the configured bps value and a little bit enhancement since there are
no hung(oversized) bios any more.

blkio.throttle.read_bps_device = 20MB/s
fio ... -numjobs=8 ...

    before : IOPS=37.9k, BW=148MiB/s (155MB/s)(11.6GiB/80333msec)
    after  : IOPS=39.0k, BW=153MiB/s (160MB/s)(15.6GiB/104914msec)

    before : clat (usec): min=4, max=1056.6k, avg=197.23, stdev=10080.69
    after  : clat (usec): min=4, max=193481, avg=188.83, stdev=465129

    before : lat (usec): min=5, max=1056.6k, avg=200.48, stdev=10080.76
    after  : lat (usec): min=5, max=193483, avg=192.68, stdev=4651.87

blkio.throttle.read_bps_device = 30MB/s
fio ... -numjobs=8 ...

    before : IOPS=57.2k, BW=224MiB/s (234MB/s)(15.6GiB/71561msec)
    after  : IOPS=58.5k, BW=229MiB/s (240MB/s)(15.6GiB/69996msec)

    before : clat (usec): min=4, max=1105.5k, avg=126.20, stdev=6419.22
    after  : clat (usec): min=4, max=183956, avg=120.60, stdev=295728

    before : lat (usec): min=5, max=1105.5k, avg=129.45, stdev=641929
    after  : lat (usec): min=5, max=183958, avg=124.40, stdev=2958.18

>
> Thanks. blk_throttle_budgt

>
> --
> tejun