Re: [PATCH] block: throttle: don't add one extra jiffy mistakenly for bps limit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

在 2025/02/21 12:18, Ming Lei 写道:
-       jiffy_wait = div64_u64(extra_bytes * HZ, bps_limit);
-
-       if (!jiffy_wait)
-               jiffy_wait = 1;
+       jiffy_wait = div64_u64_rem(extra_bytes * HZ, bps_limit,
&carryover_bytes);
+       tg->carryover_bytes[rw] -= div64_u64(carryover_bytes, HZ);
Can you explain a bit why `carryover_bytes/HZ` is subtracted instead of
carryover_bytes?

For example, if bps_limit is 1000, extra_bytes is 9, then:

jiffy_wait = (9 * 100) / 1000 = 0;
carryover_bytes = (9 * 100) % 1000 = 900;

Hence we need to divide it by HZ:
tg->carryover_bytes = 0 - 900/100 = -9;

-9 can be considered debt, and for the next IO, the bytes_allowed will
consider the carryover_bytes.

Also tg_within_bps_limit() may return 0 now, which isn't expected.

I think it's expected, this IO will now be dispatched directly instead
of wait for one more jiffies, and debt will be paid if there are
following IOs.

And if the tg idle for a long time before dispatching the next IO,
tg_trim_slice() should handle this case and avoid long slice end. This
is not quite handy, perhaps it's better to add a helper like
tg_in_debt() before throtl_start_new_slice() to hande this case.

BTW, we must update the comment about carryover_bytes/ios, it's alredy
used as debt.

Thanks,
Kuai


Thanks,
Ming





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux