linux-kernel - Re: [PATCH 04/11] blk-throttle: correct calculation of wait time in tg_may

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: <Y35kELlFkI/BtkqC@slm.duckdns.org>
Date:   Wed, 23 Nov 2022 08:18:56 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Kemeng Shi <shikemeng@...wei.com>
Cc:     josef@...icpanda.com, axboe@...nel.dk, cgroups@...r.kernel.org,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/11] blk-throttle: correct calculation of wait time in
 tg_may_dispatch

On Wed, Nov 23, 2022 at 02:03:54PM +0800, Kemeng Shi wrote:
> If bps and iops both reach limit, we always return bps wait time as
> tg_within_iops_limit is after "tg_within_bps_limit(tg, bio, bps_limit) &&"
> and will not be called if tg_within_bps_limit return true.

Maybe it's obvious but it'd be better to explain "why" this change is being
made.

> @@ -939,8 +926,9 @@ static bool tg_may_dispatch(struct throtl_grp *tg, struct bio *bio,
>  				jiffies + tg->td->throtl_slice);
>  	}
>  
> -	if (tg_within_bps_limit(tg, bio, bps_limit, &bps_wait) &&
> -	    tg_within_iops_limit(tg, bio, iops_limit, &iops_wait)) {
> +	bps_wait = tg_within_bps_limit(tg, bio, bps_limit);
> +	iops_wait = tg_within_iops_limit(tg, bio, iops_limit);
> +	if (bps_wait + iops_wait == 0) {
>  		if (wait)
>  			*wait = 0;
>  		return true;

So, max_wait is supposed to be maximum in the whole traversal path in the
tree, not just the max value in this tg, so after this, the code should be
changed to sth like the following, right?

        max_wait = max(max, max(bps_wait, iops_wait));

Thanks.

-- 
tejun