[PATCH OLK-5.10 0/3] blk-throttle: check for overflow in calculate_bytes_allowed

Check for overflow in calculate_bytes_allowed. Khazhismel Kumykov (1): blk-throttle: check for overflow in calculate_bytes_allowed Yu Kuai (2): blk-throttle: factor out code to calculate ios/bytes_allowed blk-throttle: use calculate_io/bytes_allowed() for throtl_trim_slice() block/blk-throttle.c | 97 +++++++++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 42 deletions(-) -- 2.31.1

反馈: 您发送到kernel@openeuler.org的补丁/补丁集,已成功转换为PR! PR链接地址: https://gitee.com/openeuler/kernel/pulls/9511 邮件列表地址:https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/F... FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/9511 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/F...

From: Yu Kuai <yukuai3@huawei.com> mainline inclusion from mainline-v6.1-rc1 commit 681cd46fff8cd81e387747c7850f2e730d3e0b74 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IA8QFN Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- No functional changes, new apis will be used in later patches to calculate wait time for throttled bios when new configuration is submitted. Noted this patch also rename tg_with_in_iops/bps_limit() to tg_within_iops/bps_limit(). Signed-off-by: Yu Kuai <yukuai3@huawei.com> Acked-by: Tejun Heo <tj@kernel.org> Link: https://lore.kernel.org/r/20220829022240.3348319-4-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe <axboe@kernel.dk> Conflicts: block/blk-throttle.c [Commit 34841e6fb125 ("block: revert 4f1e9630afe6 ("blk-throtl: optimize IOPS throttle for large IO scenarios")") remove the judgment of iops_limit] Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> --- block/blk-throttle.c | 59 ++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 050ddf0ad002..2464975b6863 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -913,33 +913,20 @@ static inline void throtl_trim_slice(struct throtl_grp *tg, bool rw) tg->slice_start[rw], tg->slice_end[rw], jiffies); } -static bool tg_with_in_iops_limit(struct throtl_grp *tg, struct bio *bio, - u32 iops_limit, unsigned long *wait) +static unsigned int calculate_io_allowed(u32 iops_limit, + unsigned long jiffy_elapsed) { - bool rw = bio_data_dir(bio); unsigned int io_allowed; - unsigned long jiffy_elapsed, jiffy_wait, jiffy_elapsed_rnd; u64 tmp; - if (iops_limit == UINT_MAX) { - if (wait) - *wait = 0; - return true; - } - - jiffy_elapsed = jiffies - tg->slice_start[rw]; - - /* Round up to the next throttle slice, wait time must be nonzero */ - jiffy_elapsed_rnd = roundup(jiffy_elapsed + 1, tg->td->throtl_slice); - /* - * jiffy_elapsed_rnd should not be a big value as minimum iops can be + * jiffy_elapsed should not be a big value as minimum iops can be * 1 then at max jiffy elapsed should be equivalent of 1 second as we * will allow dispatch after 1 second and after that slice should * have been trimmed. */ - tmp = (u64)iops_limit * jiffy_elapsed_rnd; + tmp = (u64)iops_limit * jiffy_elapsed; do_div(tmp, HZ); if (tmp > UINT_MAX) @@ -947,6 +934,32 @@ static bool tg_with_in_iops_limit(struct throtl_grp *tg, struct bio *bio, else io_allowed = tmp; + return io_allowed; +} + +static u64 calculate_bytes_allowed(u64 bps_limit, unsigned long jiffy_elapsed) +{ + return mul_u64_u64_div_u64(bps_limit, (u64)jiffy_elapsed, (u64)HZ); +} + +static bool tg_within_iops_limit(struct throtl_grp *tg, struct bio *bio, + u32 iops_limit, unsigned long *wait) +{ + bool rw = bio_data_dir(bio); + unsigned int io_allowed; + unsigned long jiffy_elapsed, jiffy_wait, jiffy_elapsed_rnd; + + if (iops_limit == UINT_MAX) { + if (wait) + *wait = 0; + return true; + } + + jiffy_elapsed = jiffies - tg->slice_start[rw]; + + /* Round up to the next throttle slice, wait time must be nonzero */ + jiffy_elapsed_rnd = roundup(jiffy_elapsed + 1, tg->td->throtl_slice); + io_allowed = calculate_io_allowed(iops_limit, jiffy_elapsed_rnd); if (tg->io_disp[rw] + 1 <= io_allowed) { if (wait) *wait = 0; @@ -961,8 +974,8 @@ static bool tg_with_in_iops_limit(struct throtl_grp *tg, struct bio *bio, return false; } -static bool tg_with_in_bps_limit(struct throtl_grp *tg, struct bio *bio, - u64 bps_limit, unsigned long *wait) +static bool tg_within_bps_limit(struct throtl_grp *tg, struct bio *bio, + u64 bps_limit, unsigned long *wait) { bool rw = bio_data_dir(bio); u64 bytes_allowed, extra_bytes; @@ -982,9 +995,7 @@ static bool tg_with_in_bps_limit(struct throtl_grp *tg, struct bio *bio, jiffy_elapsed_rnd = tg->td->throtl_slice; jiffy_elapsed_rnd = roundup(jiffy_elapsed_rnd, tg->td->throtl_slice); - bytes_allowed = mul_u64_u64_div_u64(bps_limit, (u64)jiffy_elapsed_rnd, - (u64)HZ); - + bytes_allowed = calculate_bytes_allowed(bps_limit, jiffy_elapsed_rnd); if (tg->bytes_disp[rw] + bio_size <= bytes_allowed) { if (wait) *wait = 0; @@ -1055,8 +1066,8 @@ static bool tg_may_dispatch(struct throtl_grp *tg, struct bio *bio, if (iops_limit != UINT_MAX) tg->io_disp[rw] += atomic_xchg(&tg->io_split_cnt[rw], 0); - if (tg_with_in_bps_limit(tg, bio, bps_limit, &bps_wait) && - tg_with_in_iops_limit(tg, bio, iops_limit, &iops_wait)) { + if (tg_within_bps_limit(tg, bio, bps_limit, &bps_wait) && + tg_within_iops_limit(tg, bio, iops_limit, &iops_wait)) { if (wait) *wait = 0; return true; -- 2.31.1

From: Yu Kuai <yukuai3@huawei.com> mainline inclusion from mainline-v6.6-rc1 commit e8368b57c006dc0e02dcd8a9dc9f2060ff5476fe category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IA8QFN Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- There are no functional changes, just make the code cleaner. Signed-off-by: Yu Kuai <yukuai3@huawei.com> Acked-by: Tejun Heo <tj@kernel.org> Link: https://lore.kernel.org/r/20230816012708.1193747-4-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe <axboe@kernel.dk> Conflicts: block/blk-throttle.c [Commit a880ae93e5b5 ("blk-throttle: fix io hung due to configuration updates") add function __tg_update_carryover()] Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> --- block/blk-throttle.c | 86 +++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 45 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 2464975b6863..379ed7981979 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -853,11 +853,40 @@ static bool throtl_slice_used(struct throtl_grp *tg, bool rw) return true; } +static unsigned int calculate_io_allowed(u32 iops_limit, + unsigned long jiffy_elapsed) +{ + unsigned int io_allowed; + u64 tmp; + + /* + * jiffy_elapsed should not be a big value as minimum iops can be + * 1 then at max jiffy elapsed should be equivalent of 1 second as we + * will allow dispatch after 1 second and after that slice should + * have been trimmed. + */ + + tmp = (u64)iops_limit * jiffy_elapsed; + do_div(tmp, HZ); + + if (tmp > UINT_MAX) + io_allowed = UINT_MAX; + else + io_allowed = tmp; + + return io_allowed; +} + +static u64 calculate_bytes_allowed(u64 bps_limit, unsigned long jiffy_elapsed) +{ + return mul_u64_u64_div_u64(bps_limit, (u64)jiffy_elapsed, (u64)HZ); +} + /* Trim the used slices and adjust slice start accordingly */ static inline void throtl_trim_slice(struct throtl_grp *tg, bool rw) { - unsigned long nr_slices, time_elapsed, io_trim; - u64 bytes_trim, tmp; + unsigned long time_elapsed, io_trim; + u64 bytes_trim; BUG_ON(time_before(tg->slice_end[rw], tg->slice_start[rw])); @@ -879,19 +908,14 @@ static inline void throtl_trim_slice(struct throtl_grp *tg, bool rw) throtl_set_slice_end(tg, rw, jiffies + tg->td->throtl_slice); - time_elapsed = jiffies - tg->slice_start[rw]; - - nr_slices = time_elapsed / tg->td->throtl_slice; - - if (!nr_slices) + time_elapsed = rounddown(jiffies - tg->slice_start[rw], + tg->td->throtl_slice); + if (!time_elapsed) return; - tmp = tg_bps_limit(tg, rw) * tg->td->throtl_slice * nr_slices; - do_div(tmp, HZ); - bytes_trim = tmp; - - io_trim = (tg_iops_limit(tg, rw) * tg->td->throtl_slice * nr_slices) / - HZ; + bytes_trim = calculate_bytes_allowed(tg_bps_limit(tg, rw), + time_elapsed); + io_trim = calculate_io_allowed(tg_iops_limit(tg, rw), time_elapsed); if (!bytes_trim && !io_trim) return; @@ -905,41 +929,13 @@ static inline void throtl_trim_slice(struct throtl_grp *tg, bool rw) else tg->io_disp[rw] = 0; - tg->slice_start[rw] += nr_slices * tg->td->throtl_slice; + tg->slice_start[rw] += time_elapsed; throtl_log(&tg->service_queue, "[%c] trim slice nr=%lu bytes=%llu io=%lu start=%lu end=%lu jiffies=%lu", - rw == READ ? 'R' : 'W', nr_slices, bytes_trim, io_trim, - tg->slice_start[rw], tg->slice_end[rw], jiffies); -} - -static unsigned int calculate_io_allowed(u32 iops_limit, - unsigned long jiffy_elapsed) -{ - unsigned int io_allowed; - u64 tmp; - - /* - * jiffy_elapsed should not be a big value as minimum iops can be - * 1 then at max jiffy elapsed should be equivalent of 1 second as we - * will allow dispatch after 1 second and after that slice should - * have been trimmed. - */ - - tmp = (u64)iops_limit * jiffy_elapsed; - do_div(tmp, HZ); - - if (tmp > UINT_MAX) - io_allowed = UINT_MAX; - else - io_allowed = tmp; - - return io_allowed; -} - -static u64 calculate_bytes_allowed(u64 bps_limit, unsigned long jiffy_elapsed) -{ - return mul_u64_u64_div_u64(bps_limit, (u64)jiffy_elapsed, (u64)HZ); + rw == READ ? 'R' : 'W', time_elapsed / tg->td->throtl_slice, + bytes_trim, io_trim, tg->slice_start[rw], tg->slice_end[rw], + jiffies); } static bool tg_within_iops_limit(struct throtl_grp *tg, struct bio *bio, -- 2.31.1

From: Khazhismel Kumykov <khazhy@chromium.org> mainline inclusion from mainline-v6.7-rc1 commit 2dd710d476f2f1f6eaca884f625f69ef4389ed40 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IA8QFN Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- Inexact, we may reject some not-overflowing values incorrectly, but they'll be on the order of exabytes allowed anyways. This fixes divide error crash on x86 if bps_limit is not configured or is set too high in the rare case that jiffy_elapsed is greater than HZ. Fixes: e8368b57c006 ("blk-throttle: use calculate_io/bytes_allowed() for throtl_trim_slice()") Fixes: 8d6bbaada2e0 ("blk-throttle: prevent overflow while calculating wait time") Signed-off-by: Khazhismel Kumykov <khazhy@google.com> Acked-by: Tejun Heo <tj@kernel.org> Link: https://lore.kernel.org/r/20231020223617.2739774-1-khazhy@google.com Signed-off-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> --- block/blk-throttle.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 379ed7981979..5475aa477bc5 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -879,6 +879,12 @@ static unsigned int calculate_io_allowed(u32 iops_limit, static u64 calculate_bytes_allowed(u64 bps_limit, unsigned long jiffy_elapsed) { + /* + * Can result be wider than 64 bits? + * We check against 62, not 64, due to ilog2 truncation. + */ + if (ilog2(bps_limit) + ilog2(jiffy_elapsed) - ilog2(HZ) > 62) + return U64_MAX; return mul_u64_u64_div_u64(bps_limit, (u64)jiffy_elapsed, (u64)HZ); } -- 2.31.1
participants (2)
-
Li Lingfeng
-
patchwork bot