linux-kernel - Re: [PATCH 2/2] blk-throttle: fix off-by-one jiffies wait

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: <dc2b3a40-b33b-0bc5-3a73-18b288b4283f@huaweicloud.com>
Date: Tue, 25 Feb 2025 19:09:30 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Ming Lei <ming.lei@...hat.com>, Yu Kuai <yukuai1@...weicloud.com>
Cc: tj@...nel.org, josef@...icpanda.com, axboe@...nel.dk,
 cgroups@...r.kernel.org, linux-block@...r.kernel.org,
 linux-kernel@...r.kernel.org, yi.zhang@...wei.com, yangerkun@...wei.com,
 "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH 2/2] blk-throttle: fix off-by-one jiffies wait_time

Hi,

在 2025/02/25 16:21, Ming Lei 写道:
> On Tue, Feb 25, 2025 at 11:12:24AM +0800, Yu Kuai wrote:
>> Hi, Ming!
>>
>> 在 2025/02/25 10:28, Ming Lei 写道:
>>> Can you explain in details why it signals that the rate is expected now?
>>>
>>> If rate isn't expected, it will cause trouble to trim, even just the
>>> previous part.
>>
>> Ok, for example, assume bps_limit is 1000bytes, 1 jiffes is 10ms, and
>> slice is 20ms(2 jiffies).
>>
> 
> We all know how it works, but I didn't understand the behind idea why it
> is correct. Now I figured it out:
> 
> 1) increase default slice window to 2 * td->throttle_slice
> 
> 2) slice window is set as [jiffies - td->throttle_slice, jiffies + td->throttle_slice]
> 
> 3) initialize td->bytes_disp[]/td->io_dis[] as actual dispatched bytes/ios
> done [jiffies - td->throttle_slice, 0]
> 
> This approach looks smart, and it should work well for any deviation which is <= 1
> throttle_slice.
> 
> Probably it is enough for fixing the issue in throtl/001, even though 2 jiffies
> timer drift still may be observed, see the below log collected in my VM(HZ_100)
> by just running one time of blktests './check throtl':
> 
> @timer_expire_delay:
> [1, 2)               387 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
> [2, 3)                11 |@                                                   |
> 
> bpftrace -e 'kfunc:throtl_pending_timer_fn { @timer_expire_delay = lhist(jiffies - args->t->expires, 0, 16, 1);}'
> 
> 
> Also I'd suggest to remove ->carryover_bytes/ios since blk-throttle algorithm is
> supposed to be adaptive, and the approach I suggested may cover this area,
> what do you think of this cleanup? I have one local patchset, which can
> pass all blktest throtl tests with removing ->carryover_bytes/ios.
> 

It's always welcome for such cleanup. BTW, do you have plans to support
bio merge for iops limit in blk-throttle? Since bio split is handled. I
was thinking about using carryover_ios, perhaps you can handle this as
well.

Thanks,
Kuai
> 
> 
> Thanks,
> Ming
> 
> 
> .
>