[PATCH 0/2] blk-throttle: minor fixes and cleanup

[PATCH 0/2] blk-throttle: minor fixes and cleanup

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

This series contains two minor patches for blk-throttle.

Patch1: simple cleanup
Patch2: fix timer scheduled on tg_flush_bios()

Zizhi Wo (2):
  blk-throttle: remove leftover carryover checks in
    tg_within_[bps/iops]_limit
  blk-throttle: fix timer scheduled on wrong service_queue in
    tg_flush_bios

 block/blk-throttle.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

-- 
2.39.2

[PATCH 1/2] blk-throttle: remove leftover carryover checks in tg_within_[bps/iops]_limit

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

Commit bb8d5587bdc3 ("blk-throttle: fix wrong comparation while
'carryover_ios/bytes' is negative") changed the type of "io_allowed" and
"bytes_allowed", and added "io_allowed > 0" and "bytes_allowed > 0" checks
to handle the case where carryover_ios/bytes could be negative.

Since commit 6cc477c36875 ("blk-throttle: carry over directly") removed
the "carryover" fields, the allowed values are now computed solely by
calculate_io_allowed() and calculate_bytes_allowed(), which always return
non-negative results. The extra checks are therefore no longer necessary.

Remove the now-unnecessary "> 0" checks and restore the types of
"io_allowed" and "bytes_allowed" to unsigned to match the return types of
the corresponding calculate functions.

Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
 block/blk-throttle.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 97188a795848..b82da0dfa8e8 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -760,7 +760,7 @@ static unsigned long tg_within_iops_limit(struct throtl_grp *tg, struct bio *bio
 				 u32 iops_limit)
 {
 	bool rw = bio_data_dir(bio);
-	int io_allowed;
+	unsigned int io_allowed;
 	unsigned long jiffy_elapsed, jiffy_wait, jiffy_elapsed_rnd;
 
 	jiffy_elapsed = jiffies - tg->slice_start[rw];
@@ -768,7 +768,7 @@ static unsigned long tg_within_iops_limit(struct throtl_grp *tg, struct bio *bio
 	/* Round up to the next throttle slice, wait time must be nonzero */
 	jiffy_elapsed_rnd = roundup(jiffy_elapsed + 1, DFL_THROTL_SLICE);
 	io_allowed = calculate_io_allowed(iops_limit, jiffy_elapsed_rnd);
-	if (io_allowed > 0 && tg->io_disp[rw] + 1 <= io_allowed)
+	if (tg->io_disp[rw] + 1 <= io_allowed)
 		return 0;
 
 	/* Calc approx time to dispatch */
@@ -783,8 +783,7 @@ static unsigned long tg_within_bps_limit(struct throtl_grp *tg, struct bio *bio,
 				u64 bps_limit)
 {
 	bool rw = bio_data_dir(bio);
-	long long bytes_allowed;
-	u64 extra_bytes;
+	u64 bytes_allowed, extra_bytes;
 	unsigned long jiffy_elapsed, jiffy_wait, jiffy_elapsed_rnd;
 	unsigned int bio_size = throtl_bio_data_size(bio);
 
@@ -796,9 +795,7 @@ static unsigned long tg_within_bps_limit(struct throtl_grp *tg, struct bio *bio,
 
 	jiffy_elapsed_rnd = roundup(jiffy_elapsed_rnd, DFL_THROTL_SLICE);
 	bytes_allowed = calculate_bytes_allowed(bps_limit, jiffy_elapsed_rnd);
-	/* Need to consider the case of bytes_allowed overflow. */
-	if ((bytes_allowed > 0 && tg->bytes_disp[rw] + bio_size <= bytes_allowed)
-	    || bytes_allowed < 0)
+	if (tg->bytes_disp[rw] + bio_size <= bytes_allowed)
 		return 0;
 
 	/* Calc approx time to dispatch */
-- 
2.39.2

Re: [PATCH 1/2] blk-throttle: remove leftover carryover checks in tg_within_[bps/iops]_limit

[Zizhi Wo]

Hi,

I need to correct myself — this change is actually problematic. Please 
ignore this patch...

[bytes/io]_disp is intentionally signed and can be negative to represent
carryover. By changing io_allowed to unsigned int, the comparison
tg->io_disp[rw] + 1 <= io_allowed becomes a signed/unsigned comparison. 
When io_disp is negative, it will be implicitly converted to a large 
unsigned value.


在 2026/3/16 16:44, Zizhi Wo 写道:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> Commit bb8d5587bdc3 ("blk-throttle: fix wrong comparation while
> 'carryover_ios/bytes' is negative") changed the type of "io_allowed" and
> "bytes_allowed", and added "io_allowed > 0" and "bytes_allowed > 0" checks
> to handle the case where carryover_ios/bytes could be negative.
> 
> Since commit 6cc477c36875 ("blk-throttle: carry over directly") removed
> the "carryover" fields, the allowed values are now computed solely by
> calculate_io_allowed() and calculate_bytes_allowed(), which always return
> non-negative results. The extra checks are therefore no longer necessary.
> 
> Remove the now-unnecessary "> 0" checks and restore the types of
> "io_allowed" and "bytes_allowed" to unsigned to match the return types of
> the corresponding calculate functions.
> 
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> ---
>   block/blk-throttle.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
> index 97188a795848..b82da0dfa8e8 100644
> --- a/block/blk-throttle.c
> +++ b/block/blk-throttle.c
> @@ -760,7 +760,7 @@ static unsigned long tg_within_iops_limit(struct throtl_grp *tg, struct bio *bio
>   				 u32 iops_limit)
>   {
>   	bool rw = bio_data_dir(bio);
> -	int io_allowed;
> +	unsigned int io_allowed;
>   	unsigned long jiffy_elapsed, jiffy_wait, jiffy_elapsed_rnd;
>   
>   	jiffy_elapsed = jiffies - tg->slice_start[rw];
> @@ -768,7 +768,7 @@ static unsigned long tg_within_iops_limit(struct throtl_grp *tg, struct bio *bio
>   	/* Round up to the next throttle slice, wait time must be nonzero */
>   	jiffy_elapsed_rnd = roundup(jiffy_elapsed + 1, DFL_THROTL_SLICE);
>   	io_allowed = calculate_io_allowed(iops_limit, jiffy_elapsed_rnd);
> -	if (io_allowed > 0 && tg->io_disp[rw] + 1 <= io_allowed)
> +	if (tg->io_disp[rw] + 1 <= io_allowed)
>   		return 0;
>   
>   	/* Calc approx time to dispatch */
> @@ -783,8 +783,7 @@ static unsigned long tg_within_bps_limit(struct throtl_grp *tg, struct bio *bio,
>   				u64 bps_limit)
>   {
>   	bool rw = bio_data_dir(bio);
> -	long long bytes_allowed;
> -	u64 extra_bytes;
> +	u64 bytes_allowed, extra_bytes;
>   	unsigned long jiffy_elapsed, jiffy_wait, jiffy_elapsed_rnd;
>   	unsigned int bio_size = throtl_bio_data_size(bio);
>   
> @@ -796,9 +795,7 @@ static unsigned long tg_within_bps_limit(struct throtl_grp *tg, struct bio *bio,
>   
>   	jiffy_elapsed_rnd = roundup(jiffy_elapsed_rnd, DFL_THROTL_SLICE);
>   	bytes_allowed = calculate_bytes_allowed(bps_limit, jiffy_elapsed_rnd);
> -	/* Need to consider the case of bytes_allowed overflow. */
> -	if ((bytes_allowed > 0 && tg->bytes_disp[rw] + bio_size <= bytes_allowed)
> -	    || bytes_allowed < 0)
> +	if (tg->bytes_disp[rw] + bio_size <= bytes_allowed)
>   		return 0;
>   
>   	/* Calc approx time to dispatch */

[PATCH 2/2] blk-throttle: fix timer scheduled on wrong service_queue in tg_flush_bios

[Zizhi Wo]

From: Zizhi Wo <wozizhi@huawei.com>

tg_flush_bios() calls tg_update_disptime() which inserts the tg into
the parent_sq's pending_tree, but then schedules the pending timer on
the tg's own sq. For leaf tg's with no children, the timer fires on an
empty pending_tree and does nothing, leaving the bio stranded until the
parent_sq's timer happens to fire naturally.

This is masked in blk_throtl_cancel_bios() which walks all levels in
post-order, but affects throtl_pd_offline() where a single leaf cgroup
goes offline.

Fix by scheduling the timer on the parent_sq where the tg was actually
enqueued.

Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
 block/blk-throttle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index b82da0dfa8e8..93e9e0e352f0 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1646,7 +1646,7 @@ static void tg_flush_bios(struct throtl_grp *tg)
 	 */
 	tg_update_disptime(tg);
 
-	throtl_schedule_pending_timer(sq, jiffies + 1);
+	throtl_schedule_pending_timer(sq->parent_sq, jiffies + 1);
 }
 
 static void throtl_pd_offline(struct blkg_policy_data *pd)
-- 
2.39.2

Re: [PATCH 0/2] blk-throttle: minor fixes and cleanup

[Shinichiro Kawasaki]

On Mar 16, 2026 / 16:44, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
> 
> This series contains two minor patches for blk-throttle.
> 
> Patch1: simple cleanup
> Patch2: fix timer scheduled on tg_flush_bios()
> 
> Zizhi Wo (2):
>   blk-throttle: remove leftover carryover checks in
>     tg_within_[bps/iops]_limit
>   blk-throttle: fix timer scheduled on wrong service_queue in
>     tg_flush_bios

Hello ZiZhi, just FYI. Blktests CI trial run found that this series triggered
the failure of the test case throtl/004 with sdebug:

throtl/004 (nullb) (delete disk while IO is throttled)      
throtl/004 (nullb) (delete disk while IO is throttled)       [passed]
    runtime    ...  1.667s
throtl/004 (sdebug) (delete disk while IO is throttled)     
throtl/004 (sdebug) (delete disk while IO is throttled)      [failed]
    runtime    ...  3.273s
    --- tests/throtl/004.out	2026-03-16 10:29:46.761897543 +0000
    +++ /home/fedora/blktests/results/nodev_sdebug/throtl/004.out.bad	2026-03-16 13:31:54.355173644 +0000
    @@ -1,3 +1,2 @@
     Running throtl/004
    -Input/output error
     Test complete

Re: [PATCH 0/2] blk-throttle: minor fixes and cleanup

[Zizhi Wo]

在 2026/3/17 15:37, Shinichiro Kawasaki 写道:
> On Mar 16, 2026 / 16:44, Zizhi Wo wrote:
>> From: Zizhi Wo <wozizhi@huawei.com>
>>
>> This series contains two minor patches for blk-throttle.
>>
>> Patch1: simple cleanup
>> Patch2: fix timer scheduled on tg_flush_bios()
>>
>> Zizhi Wo (2):
>>    blk-throttle: remove leftover carryover checks in
>>      tg_within_[bps/iops]_limit
>>    blk-throttle: fix timer scheduled on wrong service_queue in
>>      tg_flush_bios
> 
> Hello ZiZhi, just FYI. Blktests CI trial run found that this series triggered
> the failure of the test case throtl/004 with sdebug:
> 
> throtl/004 (nullb) (delete disk while IO is throttled)
> throtl/004 (nullb) (delete disk while IO is throttled)       [passed]
>      runtime    ...  1.667s
> throtl/004 (sdebug) (delete disk while IO is throttled)
> throtl/004 (sdebug) (delete disk while IO is throttled)      [failed]
>      runtime    ...  3.273s
>      --- tests/throtl/004.out	2026-03-16 10:29:46.761897543 +0000
>      +++ /home/fedora/blktests/results/nodev_sdebug/throtl/004.out.bad	2026-03-16 13:31:54.355173644 +0000
>      @@ -1,3 +1,2 @@
>       Running throtl/004
>      -Input/output error
>       Test complete

Hi, I'm very sorry for my oversight. I have already identified the issue
with the first patch in the patch set and replied to it myself. I will
review more carefully next time. Please ignore the first patch (although
the throtl test case did pass locally...)

I'd like to ask whether applying only the second patch can pass the
corresponding tests in your environment?

Thanks,
Zizhi Wo

Re: [PATCH 0/2] blk-throttle: minor fixes and cleanup

[Shinichiro Kawasaki]

On Mar 17, 2026 / 16:13, Zizhi Wo wrote:
> 
> 
> 在 2026/3/17 15:37, Shinichiro Kawasaki 写道:
> > On Mar 16, 2026 / 16:44, Zizhi Wo wrote:
> > > From: Zizhi Wo <wozizhi@huawei.com>
> > > 
> > > This series contains two minor patches for blk-throttle.
> > > 
> > > Patch1: simple cleanup
> > > Patch2: fix timer scheduled on tg_flush_bios()
> > > 
> > > Zizhi Wo (2):
> > >    blk-throttle: remove leftover carryover checks in
> > >      tg_within_[bps/iops]_limit
> > >    blk-throttle: fix timer scheduled on wrong service_queue in
> > >      tg_flush_bios
> > 
> > Hello ZiZhi, just FYI. Blktests CI trial run found that this series triggered
> > the failure of the test case throtl/004 with sdebug:
> > 
> > throtl/004 (nullb) (delete disk while IO is throttled)
> > throtl/004 (nullb) (delete disk while IO is throttled)       [passed]
> >      runtime    ...  1.667s
> > throtl/004 (sdebug) (delete disk while IO is throttled)
> > throtl/004 (sdebug) (delete disk while IO is throttled)      [failed]
> >      runtime    ...  3.273s
> >      --- tests/throtl/004.out	2026-03-16 10:29:46.761897543 +0000
> >      +++ /home/fedora/blktests/results/nodev_sdebug/throtl/004.out.bad	2026-03-16 13:31:54.355173644 +0000
> >      @@ -1,3 +1,2 @@
> >       Running throtl/004
> >      -Input/output error
> >       Test complete
> 
> Hi, I'm very sorry for my oversight. I have already identified the issue
> with the first patch in the patch set and replied to it myself. I will
> review more carefully next time. Please ignore the first patch (although
> the throtl test case did pass locally...)

Thanks for the explanation, no problem for me.

> 
> I'd like to ask whether applying only the second patch can pass the
> corresponding tests in your environment?

Sure, I ran throtl/004 on the kernel v7.0-rc4 + the second patch "blk-throttle:
fix timer scheduled on wrong service_queue in tg_flush_bios" only.
Unfortunately, I still see the test case fails for scsi_debug.

Re: [PATCH 0/2] blk-throttle: minor fixes and cleanup

[Zizhi Wo]

在 2026/3/17 19:41, Shinichiro Kawasaki 写道:
> On Mar 17, 2026 / 16:13, Zizhi Wo wrote:
>>
>>
>> 在 2026/3/17 15:37, Shinichiro Kawasaki 写道:
>>> On Mar 16, 2026 / 16:44, Zizhi Wo wrote:
>>>> From: Zizhi Wo <wozizhi@huawei.com>
>>>>
>>>> This series contains two minor patches for blk-throttle.
>>>>
>>>> Patch1: simple cleanup
>>>> Patch2: fix timer scheduled on tg_flush_bios()
>>>>
>>>> Zizhi Wo (2):
>>>>     blk-throttle: remove leftover carryover checks in
>>>>       tg_within_[bps/iops]_limit
>>>>     blk-throttle: fix timer scheduled on wrong service_queue in
>>>>       tg_flush_bios
>>>
>>> Hello ZiZhi, just FYI. Blktests CI trial run found that this series triggered
>>> the failure of the test case throtl/004 with sdebug:
>>>
>>> throtl/004 (nullb) (delete disk while IO is throttled)
>>> throtl/004 (nullb) (delete disk while IO is throttled)       [passed]
>>>       runtime    ...  1.667s
>>> throtl/004 (sdebug) (delete disk while IO is throttled)
>>> throtl/004 (sdebug) (delete disk while IO is throttled)      [failed]
>>>       runtime    ...  3.273s
>>>       --- tests/throtl/004.out	2026-03-16 10:29:46.761897543 +0000
>>>       +++ /home/fedora/blktests/results/nodev_sdebug/throtl/004.out.bad	2026-03-16 13:31:54.355173644 +0000
>>>       @@ -1,3 +1,2 @@
>>>        Running throtl/004
>>>       -Input/output error
>>>        Test complete
>>
>> Hi, I'm very sorry for my oversight. I have already identified the issue
>> with the first patch in the patch set and replied to it myself. I will
>> review more carefully next time. Please ignore the first patch (although
>> the throtl test case did pass locally...)
> 
> Thanks for the explanation, no problem for me.
> 
>>
>> I'd like to ask whether applying only the second patch can pass the
>> corresponding tests in your environment?
> 
> Sure, I ran throtl/004 on the kernel v7.0-rc4 + the second patch "blk-throttle:
> fix timer scheduled on wrong service_queue in tg_flush_bios" only.
> Unfortunately, I still see the test case fails for scsi_debug.

Oh, that is strange. I wasn't able to reproduce it locally. Based on
theoretical analysis, the tg should be placed into
sq->parent_sq->pending_tree for scheduling (tg_service_queue_add), and I
haven't been able to identify the root cause :(

__del_gendisk() will set GD_DEAD first, then call
blk_throtl_cancel_bios->tg_flush_bios; bio_io_error() will set EIO. So
I'm not sure why the current test case doesn't show an "Input/output
error" — it seems like the bio was submitted early or got lost? There
may be some cases I haven't considered. In any case, thanks for
providing the test.

Thanks,
Zizhi Wo