linux-kernel - Re: [PATCH v7 1/9] blk-throttle: fix that io throttle can only work for single bio

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yv0qbDR+cxKeZ3nD@slm.duckdns.org>
Date:   Wed, 17 Aug 2022 07:50:36 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Yu Kuai <yukuai1@...weicloud.com>
Cc:     mkoutny@...e.com, axboe@...nel.dk, ming.lei@...hat.com,
        cgroups@...r.kernel.org, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, yi.zhang@...wei.com,
        "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH v7 1/9] blk-throttle: fix that io throttle can only work
 for single bio

Hello,

On Wed, Aug 17, 2022 at 09:13:38AM +0800, Yu Kuai wrote:
> > So, as a fix for the immediate problem, I guess this might do but this feels
> > really fragile. How can we be certain that re-entering only happens because
> > of splitting? What if future core development changes that? It seems to be
> > solving the problem in the wrong place. Shouldn't we flag the bio indicating
> > that it's split when we're splitting the bio so that we only limit them for
> > iops in the first place?
>
> Splited bio is tracked in __bio_clone:

As the word is used in commit messages and comments, the past perfect form
of the verb "split" is "split". It looks like "splitted" is used in rare
cases but dictionary says it's an archaic form.

> if (bio_flagged(bio_src, BIO_THROTTLED))
> 	bio_set_flag(bio, BIO_THROTTLED);
> 
> And currenty, the iops limit and bps limit are treated differently,
> however there are only one flag 'BIO_THROTTLED' and they can't be
> distinguished.
> 
> Perhaps I can use two flags, for example BIO_IOPS_THROTTLED and
> BIO_BPS_THROTTLED, this way only iops limit can be handled and bps
> limit can be skipped for splited bio.
> 
> What do you think?

I think the code would be a lot more intuitive and less fragile if we used
two flags but the bits in the bi_flags field are a scarce resource
unfortunately. Even then, I think the right thing to do here is using two
flags.

Thanks.

-- 
tejun