hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

--------------------------------

This reverts commit 510d96b0f5a1edbd9ed1e9efbfde87930a3588b3.

Prepare to backport mainline solution in the next patch instead.

Fixes: 510d96b0f5a1 ("blk-throttle: fix io hung due to configuration updates") Signed-off-by: Yu Kuai yukuai3@huawei.com --- block/blk-throttle.c | 79 ++++++++------------------------------------ 1 file changed, 13 insertions(+), 66 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 5475aa477bc5..4e5cfc1c79e0 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1432,57 +1432,7 @@ static int tg_print_conf_uint(struct seq_file *sf, void *v) return 0; }

-static u64 throtl_update_bytes_disp(u64 dispatched, u64 new_limit, - u64 old_limit) -{ - if (new_limit == old_limit) - return dispatched; - - if (!dispatched) - return 0; - - /* - * In the case that multiply will overflow, just return 0. It will only - * let bios to be dispatched earlier. - */ - if (div64_u64(U64_MAX, dispatched) < new_limit) - return 0; - - dispatched *= new_limit; - return div64_u64(dispatched, old_limit); -} - -static u32 throtl_update_io_disp(u32 dispatched, u32 new_limit, u32 old_limit) -{ - if (new_limit == old_limit) - return dispatched; - - if (!dispatched) - return 0; - /* - * In the case that multiply will overflow, just return 0. It will only - * let bios to be dispatched earlier. - */ - if (UINT_MAX / dispatched < new_limit) - return 0; - - dispatched *= new_limit; - return dispatched / old_limit; -} - -static void throtl_update_slice(struct throtl_grp *tg, u64 *old_limits) -{ - tg->bytes_disp[READ] = throtl_update_bytes_disp(tg->bytes_disp[READ], - tg_bps_limit(tg, READ), old_limits[0]); - tg->bytes_disp[WRITE] = throtl_update_bytes_disp(tg->bytes_disp[WRITE], - tg_bps_limit(tg, WRITE), old_limits[1]); - tg->io_disp[READ] = throtl_update_io_disp(tg->io_disp[READ], - tg_iops_limit(tg, READ), (u32)old_limits[2]); - tg->io_disp[WRITE] = throtl_update_io_disp(tg->io_disp[WRITE], - tg_iops_limit(tg, WRITE), (u32)old_limits[3]); -} - -static void tg_conf_updated(struct throtl_grp *tg, u64 *old_limits, bool global) +static void tg_conf_updated(struct throtl_grp *tg, bool global) { struct throtl_service_queue *sq = &tg->service_queue; struct cgroup_subsys_state *pos_css; @@ -1523,7 +1473,16 @@ static void tg_conf_updated(struct throtl_grp *tg, u64 *old_limits, bool global) } rcu_read_unlock();

- throtl_update_slice(tg, old_limits); + /* + * We're already holding queue_lock and know @tg is valid. Let's + * apply the new config directly. + * + * Restart the slices for both READ and WRITES. It might happen + * that a group's limit are dropped suddenly and we don't want to + * account recently dispatched IO with new low rate. + */ + throtl_start_new_slice(tg, READ); + throtl_start_new_slice(tg, WRITE);

if (tg->flags & THROTL_TG_PENDING) { tg_update_disptime(tg); @@ -1556,14 +1515,6 @@ static inline int throtl_restart_syscall_when_busy(int errno) return ret; }

-static void tg_get_limits(struct throtl_grp *tg, u64 *limits) -{ - limits[0] = tg_bps_limit(tg, READ); - limits[1] = tg_bps_limit(tg, WRITE); - limits[2] = tg_iops_limit(tg, READ); - limits[3] = tg_iops_limit(tg, WRITE); -} - static ssize_t tg_set_conf(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off, bool is_u64) { @@ -1572,7 +1523,6 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of, struct throtl_grp *tg; int ret; u64 v; - u64 old_limits[4];

ret = blkg_conf_prep(blkcg, &blkcg_policy_throtl, buf, &ctx); if (ret) @@ -1589,14 +1539,13 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of, v = U64_MAX;

tg = blkg_to_tg(ctx.blkg); - tg_get_limits(tg, old_limits);

if (is_u64) *(u64 *)((void *)tg + of_cft(of)->private) = v; else *(unsigned int *)((void *)tg + of_cft(of)->private) = v;

- tg_conf_updated(tg, old_limits, false); + tg_conf_updated(tg, false); ret = 0; out_finish: blkg_conf_finish(&ctx); @@ -1767,7 +1716,6 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, struct blkg_conf_ctx ctx; struct throtl_grp *tg; u64 v[4]; - u64 old_limits[4]; unsigned long idle_time; unsigned long latency_time; int ret; @@ -1786,7 +1734,6 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, v[1] = tg->bps_conf[WRITE][index]; v[2] = tg->iops_conf[READ][index]; v[3] = tg->iops_conf[WRITE][index]; - tg_get_limits(tg, old_limits);

idle_time = tg->idletime_threshold_conf; latency_time = tg->latency_target_conf; @@ -1873,7 +1820,7 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, tg->td->limit_index = LIMIT_LOW; } else tg->td->limit_index = LIMIT_MAX; - tg_conf_updated(tg, old_limits, index == LIMIT_LOW && + tg_conf_updated(tg, index == LIMIT_LOW && tg->td->limit_valid[LIMIT_LOW]); ret = 0; out_finish:

From: Jens Axboe axboe@kernel.dk

mainline inclusion from mainline-v5.16-rc1 commit a7b36ee6ba299ffa5c3b36af187b4d0fb32a557c category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

Even if no policies are defined, we spend ~2% of the total IO time checking. Move the fast path inline.

Acked-by: Tejun Heo tj@kernel.org Signed-off-by: Jens Axboe axboe@kernel.dk Conflicts: block/blk-throttle.c block/blk-cgroup.c block/blk-core.c block/blk-merge.c [Contexts conflicts, and hide blk-throttle.h in blk-merge.c to prevent kabi broken.] Signed-off-by: Yu Kuai yukuai3@huawei.com --- block/blk-cgroup.c | 1 + block/blk-core.c | 1 + block/blk-merge.c | 4 + block/blk-sysfs.c | 1 + block/blk-throttle.c | 161 +------------------------------------- block/blk-throttle.h | 182 +++++++++++++++++++++++++++++++++++++++++++ block/blk.h | 16 ---- 7 files changed, 192 insertions(+), 174 deletions(-) create mode 100644 block/blk-throttle.h

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 1f2c93e9daa1..0872b392360d 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -31,6 +31,7 @@ #include <linux/tracehook.h> #include <linux/psi.h> #include "blk.h" +#include "blk-throttle.h"

#define MAX_KEY_LEN 100

diff --git a/block/blk-core.c b/block/blk-core.c index bf3bfc3ed339..2f8829bc7e50 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -51,6 +51,7 @@ #include "blk-mq-sched.h" #include "blk-pm.h" #include "blk-rq-qos.h" +#include "blk-throttle.h"

struct dentry *blk_debugfs_root;

diff --git a/block/blk-merge.c b/block/blk-merge.c index 3b2004308e93..f5c770f57946 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -13,6 +13,10 @@ #include "blk.h" #include "blk-rq-qos.h"

+#ifndef __GENKSYMS__ +#include "blk-throttle.h" +#endif + /* * rq_straddles_atomic_write_boundary - check for boundary violation * @rq: request to check diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 078aace75204..bfca6fb243d3 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -17,6 +17,7 @@ #include "blk-mq.h" #include "blk-mq-debugfs.h" #include "blk-wbt.h" +#include "blk-throttle.h"

struct queue_sysfs_entry { struct attribute attr; diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 8a22fd8449bf..dc2e2db6013a 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -14,6 +14,7 @@ #include <linux/delay.h> #include "blk.h" #include "blk-cgroup-rwstat.h" +#include "blk-throttle.h"

/* Max dispatch from a group in 1 round */ #define THROTL_GRP_QUANTUM 8 @@ -38,8 +39,6 @@ */ #define LATENCY_FILTERED_HD (1000L) /* 1ms */

-static struct blkcg_policy blkcg_policy_throtl; - /* A workqueue to queue throttle related work */ static struct workqueue_struct *kthrotld_workqueue;

@@ -56,55 +55,6 @@ static int __init setup_global_limit(char *str)

__setup("blkcg_global_limit=", setup_global_limit);

-/* - * To implement hierarchical throttling, throtl_grps form a tree and bios - * are dispatched upwards level by level until they reach the top and get - * issued. When dispatching bios from the children and local group at each - * level, if the bios are dispatched into a single bio_list, there's a risk - * of a local or child group which can queue many bios at once filling up - * the list starving others. - * - * To avoid such starvation, dispatched bios are queued separately - * according to where they came from. When they are again dispatched to - * the parent, they're popped in round-robin order so that no single source - * hogs the dispatch window. - * - * throtl_qnode is used to keep the queued bios separated by their sources. - * Bios are queued to throtl_qnode which in turn is queued to - * throtl_service_queue and then dispatched in round-robin order. - * - * It's also used to track the reference counts on blkg's. A qnode always - * belongs to a throtl_grp and gets queued on itself or the parent, so - * incrementing the reference of the associated throtl_grp when a qnode is - * queued and decrementing when dequeued is enough to keep the whole blkg - * tree pinned while bios are in flight. - */ -struct throtl_qnode { - struct list_head node; /* service_queue->queued[] */ - struct bio_list bios; /* queued bios */ - struct throtl_grp *tg; /* tg this qnode belongs to */ -}; - -struct throtl_service_queue { - struct throtl_service_queue *parent_sq; /* the parent service_queue */ - - /* - * Bios queued directly to this service_queue or dispatched from - * children throtl_grp's. - */ - struct list_head queued[2]; /* throtl_qnode [READ/WRITE] */ - unsigned int nr_queued[2]; /* number of queued bios */ - - /* - * RB tree of active children throtl_grp's, which are sorted by - * their ->disptime. - */ - struct rb_root_cached pending_tree; /* RB tree of active tgs */ - unsigned int nr_pending; /* # queued in the tree */ - unsigned long first_pending_disptime; /* disptime of the first tg */ - struct timer_list pending_timer; /* fires on first_pending_disptime */ -}; - enum tg_state_flags { THROTL_TG_PENDING = 1 << 0, /* on parent's pending tree */ THROTL_TG_WAS_EMPTY = 1 << 1, /* bio_lists[] became non-empty */ @@ -112,93 +62,6 @@ enum tg_state_flags {

#define rb_entry_tg(node) rb_entry((node), struct throtl_grp, rb_node)

-enum { - LIMIT_LOW, - LIMIT_MAX, - LIMIT_CNT, -}; - -struct throtl_grp { - /* must be the first member */ - struct blkg_policy_data pd; - - /* active throtl group service_queue member */ - struct rb_node rb_node; - - /* throtl_data this group belongs to */ - struct throtl_data *td; - - /* this group's service queue */ - struct throtl_service_queue service_queue; - - /* - * qnode_on_self is used when bios are directly queued to this - * throtl_grp so that local bios compete fairly with bios - * dispatched from children. qnode_on_parent is used when bios are - * dispatched from this throtl_grp into its parent and will compete - * with the sibling qnode_on_parents and the parent's - * qnode_on_self. - */ - struct throtl_qnode qnode_on_self[2]; - struct throtl_qnode qnode_on_parent[2]; - - /* - * Dispatch time in jiffies. This is the estimated time when group - * will unthrottle and is ready to dispatch more bio. It is used as - * key to sort active groups in service tree. - */ - unsigned long disptime; - - unsigned int flags; - - /* are there any throtl rules between this group and td? */ - bool has_rules[2]; - - /* internally used bytes per second rate limits */ - uint64_t bps[2][LIMIT_CNT]; - /* user configured bps limits */ - uint64_t bps_conf[2][LIMIT_CNT]; - - /* internally used IOPS limits */ - unsigned int iops[2][LIMIT_CNT]; - /* user configured IOPS limits */ - unsigned int iops_conf[2][LIMIT_CNT]; - - /* Number of bytes dispatched in current slice */ - uint64_t bytes_disp[2]; - /* Number of bio's dispatched in current slice */ - unsigned int io_disp[2]; - - unsigned long last_low_overflow_time[2]; - - uint64_t last_bytes_disp[2]; - unsigned int last_io_disp[2]; - - unsigned long last_check_time; - - unsigned long latency_target; /* us */ - unsigned long latency_target_conf; /* us */ - /* When did we start a new slice */ - unsigned long slice_start[2]; - unsigned long slice_end[2]; - - unsigned long last_finish_time; /* ns / 1024 */ - unsigned long checked_last_finish_time; /* ns / 1024 */ - unsigned long avg_idletime; /* ns / 1024 */ - unsigned long idletime_threshold; /* us */ - unsigned long idletime_threshold_conf; /* us */ - - unsigned int bio_cnt; /* total bios */ - unsigned int bad_bio_cnt; /* bios exceeding latency threshold */ - unsigned long bio_cnt_reset_time; - - atomic_t io_split_cnt[2]; - atomic_t last_io_split_cnt[2]; - - struct blkg_rwstat stat_bytes; - struct blkg_rwstat stat_ios; -}; - /* We measure latency for request size from <= 4k to >= 1M */ #define LATENCY_BUCKET_SIZE 9

@@ -245,16 +108,6 @@ struct throtl_data

static void throtl_pending_timer_fn(struct timer_list *t);

-static inline struct throtl_grp *pd_to_tg(struct blkg_policy_data *pd) -{ - return pd ? container_of(pd, struct throtl_grp, pd) : NULL; -} - -static inline struct throtl_grp *blkg_to_tg(struct blkcg_gq *blkg) -{ - return pd_to_tg(blkg_to_pd(blkg, &blkcg_policy_throtl)); -} - static inline struct blkcg_gq *tg_to_blkg(struct throtl_grp *tg) { return pd_to_blkg(&tg->pd); @@ -1856,7 +1709,7 @@ static void throtl_shutdown_wq(struct request_queue *q) cancel_work_sync(&td->dispatch_work); }

-static struct blkcg_policy blkcg_policy_throtl = { +struct blkcg_policy blkcg_policy_throtl = { .dfl_cftypes = throtl_files, .legacy_cftypes = throtl_legacy_files,

@@ -2270,7 +2123,7 @@ void blk_throtl_charge_bio_split(struct bio *bio) } while (parent); }

-bool blk_throtl_bio(struct bio *bio) +bool __blk_throtl_bio(struct bio *bio) { struct request_queue *q = bio->bi_disk->queue; struct blkcg_gq *blkg = bio->bi_blkg; @@ -2283,19 +2136,12 @@ bool blk_throtl_bio(struct bio *bio)

rcu_read_lock();

- /* see throtl_charge_bio() */ - if (bio_flagged(bio, BIO_THROTTLED)) - goto out; - if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) { blkg_rwstat_add(&tg->stat_bytes, bio->bi_opf, bio->bi_iter.bi_size); blkg_rwstat_add(&tg->stat_ios, bio->bi_opf, 1); }

- if (!tg->has_rules[rw]) - goto out; - spin_lock_irq(&q->queue_lock);

throtl_update_latency_buckets(td); @@ -2379,7 +2225,6 @@ bool blk_throtl_bio(struct bio *bio)

out_unlock: spin_unlock_irq(&q->queue_lock); -out: bio_set_flag(bio, BIO_THROTTLED);

#ifdef CONFIG_BLK_DEV_THROTTLING_LOW diff --git a/block/blk-throttle.h b/block/blk-throttle.h new file mode 100644 index 000000000000..175f03abd9e4 --- /dev/null +++ b/block/blk-throttle.h @@ -0,0 +1,182 @@ +#ifndef BLK_THROTTLE_H +#define BLK_THROTTLE_H + +#include "blk-cgroup-rwstat.h" + +/* + * To implement hierarchical throttling, throtl_grps form a tree and bios + * are dispatched upwards level by level until they reach the top and get + * issued. When dispatching bios from the children and local group at each + * level, if the bios are dispatched into a single bio_list, there's a risk + * of a local or child group which can queue many bios at once filling up + * the list starving others. + * + * To avoid such starvation, dispatched bios are queued separately + * according to where they came from. When they are again dispatched to + * the parent, they're popped in round-robin order so that no single source + * hogs the dispatch window. + * + * throtl_qnode is used to keep the queued bios separated by their sources. + * Bios are queued to throtl_qnode which in turn is queued to + * throtl_service_queue and then dispatched in round-robin order. + * + * It's also used to track the reference counts on blkg's. A qnode always + * belongs to a throtl_grp and gets queued on itself or the parent, so + * incrementing the reference of the associated throtl_grp when a qnode is + * queued and decrementing when dequeued is enough to keep the whole blkg + * tree pinned while bios are in flight. + */ +struct throtl_qnode { + struct list_head node; /* service_queue->queued[] */ + struct bio_list bios; /* queued bios */ + struct throtl_grp *tg; /* tg this qnode belongs to */ +}; + +struct throtl_service_queue { + struct throtl_service_queue *parent_sq; /* the parent service_queue */ + + /* + * Bios queued directly to this service_queue or dispatched from + * children throtl_grp's. + */ + struct list_head queued[2]; /* throtl_qnode [READ/WRITE] */ + unsigned int nr_queued[2]; /* number of queued bios */ + + /* + * RB tree of active children throtl_grp's, which are sorted by + * their ->disptime. + */ + struct rb_root_cached pending_tree; /* RB tree of active tgs */ + unsigned int nr_pending; /* # queued in the tree */ + unsigned long first_pending_disptime; /* disptime of the first tg */ + struct timer_list pending_timer; /* fires on first_pending_disptime */ +}; + +enum { + LIMIT_LOW, + LIMIT_MAX, + LIMIT_CNT, +}; + +struct throtl_grp { + /* must be the first member */ + struct blkg_policy_data pd; + + /* active throtl group service_queue member */ + struct rb_node rb_node; + + /* throtl_data this group belongs to */ + struct throtl_data *td; + + /* this group's service queue */ + struct throtl_service_queue service_queue; + + /* + * qnode_on_self is used when bios are directly queued to this + * throtl_grp so that local bios compete fairly with bios + * dispatched from children. qnode_on_parent is used when bios are + * dispatched from this throtl_grp into its parent and will compete + * with the sibling qnode_on_parents and the parent's + * qnode_on_self. + */ + struct throtl_qnode qnode_on_self[2]; + struct throtl_qnode qnode_on_parent[2]; + + /* + * Dispatch time in jiffies. This is the estimated time when group + * will unthrottle and is ready to dispatch more bio. It is used as + * key to sort active groups in service tree. + */ + unsigned long disptime; + + unsigned int flags; + + /* are there any throtl rules between this group and td? */ + bool has_rules[2]; + + /* internally used bytes per second rate limits */ + uint64_t bps[2][LIMIT_CNT]; + /* user configured bps limits */ + uint64_t bps_conf[2][LIMIT_CNT]; + + /* internally used IOPS limits */ + unsigned int iops[2][LIMIT_CNT]; + /* user configured IOPS limits */ + unsigned int iops_conf[2][LIMIT_CNT]; + + /* Number of bytes dispatched in current slice */ + uint64_t bytes_disp[2]; + /* Number of bio's dispatched in current slice */ + unsigned int io_disp[2]; + + unsigned long last_low_overflow_time[2]; + + uint64_t last_bytes_disp[2]; + unsigned int last_io_disp[2]; + + unsigned long last_check_time; + + unsigned long latency_target; /* us */ + unsigned long latency_target_conf; /* us */ + /* When did we start a new slice */ + unsigned long slice_start[2]; + unsigned long slice_end[2]; + + unsigned long last_finish_time; /* ns / 1024 */ + unsigned long checked_last_finish_time; /* ns / 1024 */ + unsigned long avg_idletime; /* ns / 1024 */ + unsigned long idletime_threshold; /* us */ + unsigned long idletime_threshold_conf; /* us */ + + unsigned int bio_cnt; /* total bios */ + unsigned int bad_bio_cnt; /* bios exceeding latency threshold */ + unsigned long bio_cnt_reset_time; + + atomic_t io_split_cnt[2]; + atomic_t last_io_split_cnt[2]; + + struct blkg_rwstat stat_bytes; + struct blkg_rwstat stat_ios; +}; + +extern struct blkcg_policy blkcg_policy_throtl; + +static inline struct throtl_grp *pd_to_tg(struct blkg_policy_data *pd) +{ + return pd ? container_of(pd, struct throtl_grp, pd) : NULL; +} + +static inline struct throtl_grp *blkg_to_tg(struct blkcg_gq *blkg) +{ + return pd_to_tg(blkg_to_pd(blkg, &blkcg_policy_throtl)); +} + +/* + * Internal throttling interface + */ +#ifndef CONFIG_BLK_DEV_THROTTLING +static inline int blk_throtl_init(struct request_queue *q) { return 0; } +static inline void blk_throtl_exit(struct request_queue *q) { } +static inline void blk_throtl_register_queue(struct request_queue *q) { } +static inline void blk_throtl_charge_bio_split(struct bio *bio) { } +static inline bool blk_throtl_bio(struct bio *bio) { return false; } +#else /* CONFIG_BLK_DEV_THROTTLING */ +int blk_throtl_init(struct request_queue *q); +void blk_throtl_exit(struct request_queue *q); +void blk_throtl_register_queue(struct request_queue *q); +void blk_throtl_charge_bio_split(struct bio *bio); +bool __blk_throtl_bio(struct bio *bio); +static inline bool blk_throtl_bio(struct bio *bio) +{ + struct throtl_grp *tg = blkg_to_tg(bio->bi_blkg); + + if (bio_flagged(bio, BIO_THROTTLED)) + return false; + if (!tg->has_rules[bio_data_dir(bio)]) + return false; + + return __blk_throtl_bio(bio); +} +#endif /* CONFIG_BLK_DEV_THROTTLING */ + +#endif diff --git a/block/blk.h b/block/blk.h index c86d27d80ba0..b3f0981148c9 100644 --- a/block/blk.h +++ b/block/blk.h @@ -301,22 +301,6 @@ void ioc_clear_queue(struct request_queue *q);

int create_task_io_context(struct task_struct *task, gfp_t gfp_mask, int node);

-/* - * Internal throttling interface - */ -#ifdef CONFIG_BLK_DEV_THROTTLING -extern int blk_throtl_init(struct request_queue *q); -extern void blk_throtl_exit(struct request_queue *q); -extern void blk_throtl_register_queue(struct request_queue *q); -extern void blk_throtl_charge_bio_split(struct bio *bio); -bool blk_throtl_bio(struct bio *bio); -#else /* CONFIG_BLK_DEV_THROTTLING */ -static inline int blk_throtl_init(struct request_queue *q) { return 0; } -static inline void blk_throtl_exit(struct request_queue *q) { } -static inline void blk_throtl_register_queue(struct request_queue *q) { } -static inline void blk_throtl_charge_bio_split(struct bio *bio) { } -static inline bool blk_throtl_bio(struct bio *bio) { return false; } -#endif /* CONFIG_BLK_DEV_THROTTLING */ #ifdef CONFIG_BLK_DEV_THROTTLING_LOW extern ssize_t blk_throtl_sample_time_show(struct request_queue *q, char *page); extern ssize_t blk_throtl_sample_time_store(struct request_queue *q,

From: Ming Lei ming.lei@redhat.com

mainline inclusion from mainline-v5.18-rc1 commit 5a93b6027eb4ef5db60a4bc5bdbeba5fb9f29384 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

We need to throttle split bio in case of IOPS limit even though the split bio has been marked as BIO_THROTTLED since block layer accounts split bio actually.

If only throughput throttle is setup, no need to throttle any more if BIO_THROTTLED is set since we have accounted & considered the whole bio bytes already.

Add one flag of THROTL_TG_HAS_IOPS_LIMIT for serving this purpose.

Acked-by: Tejun Heo tj@kernel.org Signed-off-by: Ming Lei ming.lei@redhat.com Link: https://lore.kernel.org/r/20220216044514.2903784-8-ming.lei@redhat.com Signed-off-by: Jens Axboe axboe@kernel.dk Conflicts: block/blk-throttle.c [Context conflicts] Signed-off-by: Yu Kuai yukuai3@huawei.com --- block/blk-throttle.c | 21 ++++++++++++++------- block/blk-throttle.h | 11 +++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 007acfb90b4d..98f3cc66674c 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -55,11 +55,6 @@ static int __init setup_global_limit(char *str)

__setup("blkcg_global_limit=", setup_global_limit);

-enum tg_state_flags { - THROTL_TG_PENDING = 1 << 0, /* on parent's pending tree */ - THROTL_TG_WAS_EMPTY = 1 << 1, /* bio_lists[] became non-empty */ -}; - #define rb_entry_tg(node) rb_entry((node), struct throtl_grp, rb_node)

/* We measure latency for request size from <= 4k to >= 1M */ @@ -440,12 +435,24 @@ static void tg_update_has_rules(struct throtl_grp *tg) struct throtl_grp *parent_tg = sq_to_tg(tg->service_queue.parent_sq); struct throtl_data *td = tg->td; int rw; + int has_iops_limit = 0; + + for (rw = READ; rw <= WRITE; rw++) { + unsigned int iops_limit = tg_iops_limit(tg, rw);

- for (rw = READ; rw <= WRITE; rw++) tg->has_rules[rw] = (parent_tg && parent_tg->has_rules[rw]) || (td->limit_valid[td->limit_index] && (tg_bps_limit(tg, rw) != U64_MAX || - tg_iops_limit(tg, rw) != UINT_MAX)); + iops_limit != UINT_MAX)); + + if (iops_limit != UINT_MAX) + has_iops_limit = 1; + } + + if (has_iops_limit) + tg->flags |= THROTL_TG_HAS_IOPS_LIMIT; + else + tg->flags &= ~THROTL_TG_HAS_IOPS_LIMIT; }

static void throtl_pd_online(struct blkg_policy_data *pd) diff --git a/block/blk-throttle.h b/block/blk-throttle.h index cb43f4417d6e..c996a15f290e 100644 --- a/block/blk-throttle.h +++ b/block/blk-throttle.h @@ -52,6 +52,12 @@ struct throtl_service_queue { struct timer_list pending_timer; /* fires on first_pending_disptime */ };

+enum tg_state_flags { + THROTL_TG_PENDING = 1 << 0, /* on parent's pending tree */ + THROTL_TG_WAS_EMPTY = 1 << 1, /* bio_lists[] became non-empty */ + THROTL_TG_HAS_IOPS_LIMIT = 1 << 2, /* tg has iops limit */ +}; + enum { LIMIT_LOW, LIMIT_MAX, @@ -170,6 +176,11 @@ static inline bool blk_throtl_bio(struct bio *bio) { struct throtl_grp *tg = blkg_to_tg(bio->bi_blkg);

+ /* no need to throttle bps any more if the bio has been throttled */ + if (bio_flagged(bio, BIO_THROTTLED) && + !(tg->flags & THROTL_TG_HAS_IOPS_LIMIT)) + return false; + if (!tg->has_rules[bio_data_dir(bio)]) return false;

From: Ming Lei ming.lei@redhat.com

mainline inclusion from mainline-v5.18-rc1 commit ee37eddbfa9e0401f13a01691cf4bbbacd2d16c9 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

In throtl_pending_timer_fn(), request queue is retrieved from throttle data. And tg's pending timer is deleted synchronously when releasing the associated blkg, at that time, throttle data may have been freed since commit 1059699f87eb ("block: move blkcg initialization/destroy into disk allocation/release handler") moves freeing q->td to disk_release() from blk_release_queue(). So use-after-free on q->td in throtl_pending_timer_fn can be triggered.

Fixes the issue by:

- do nothing in case that disk is released, when there isn't any bio to dispatch

- retrieve request queue from blkg instead of throttle data for non top-level pending timer.

Signed-off-by: Ming Lei ming.lei@redhat.com Link: https://lore.kernel.org/r/20220318130144.1066064-2-ming.lei@redhat.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Yu Kuai yukuai3@huawei.com --- block/blk-throttle.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c index fec47ad3b04e..4f73b10734ab 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1163,12 +1163,22 @@ static void throtl_pending_timer_fn(struct timer_list *t) struct throtl_service_queue *sq = from_timer(sq, t, pending_timer); struct throtl_grp *tg = sq_to_tg(sq); struct throtl_data *td = sq_to_td(sq); - struct request_queue *q = td->queue; struct throtl_service_queue *parent_sq; + struct request_queue *q; bool dispatched; int ret;

+ /* throtl_data may be gone, so figure out request queue by blkg */ + if (tg) + q = tg->pd.blkg->q; + else + q = td->queue; + spin_lock_irq(&q->queue_lock); + + if (!q->root_blkg) + goto out_unlock; + if (throtl_can_upgrade(td, NULL)) throtl_upgrade_state(td);

mainline inclusion from mainline-v6.1-rc1 commit 320fb0f91e55ba248d4bad106b408e59099cfa89 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

Test scripts: cd /sys/fs/cgroup/blkio/ echo "8:0 1024" > blkio.throttle.write_bps_device echo $$ > cgroup.procs dd if=/dev/zero of=/dev/sda bs=10k count=1 oflag=direct & dd if=/dev/zero of=/dev/sda bs=10k count=1 oflag=direct &

Test result: 10240 bytes (10 kB, 10 KiB) copied, 10.0134 s, 1.0 kB/s 10240 bytes (10 kB, 10 KiB) copied, 10.0135 s, 1.0 kB/s

The problem is that the second bio is finished after 10s instead of 20s.

Root cause: 1) second bio will be flagged:

__blk_throtl_bio while (true) { ... if (sq->nr_queued[rw]) -> some bio is throttled already break }; bio_set_flag(bio, BIO_THROTTLED); -> flag the bio

2) flagged bio will be dispatched without waiting:

throtl_dispatch_tg tg_may_dispatch tg_with_in_bps_limit if (bps_limit == U64_MAX || bio_flagged(bio, BIO_THROTTLED)) *wait = 0; -> wait time is zero return true;

commit 9f5ede3c01f9 ("block: throttle split bio in case of iops limit") support to count split bios for iops limit, thus it adds flagged bio checking in tg_with_in_bps_limit() so that split bios will only count once for bps limit, however, it introduce a new problem that io throttle won't work if multiple bios are throttled.

In order to fix the problem, handle iops/bps limit in different ways:

1) for iops limit, there is no flag to record if the bio is throttled, and iops is always applied. 2) for bps limit, original bio will be flagged with BIO_BPS_THROTTLED, and io throttle will ignore bio with the flag.

Noted this patch also remove the code to set flag in __bio_clone(), it's introduced in commit 111be8839817 ("block-throttle: avoid double charge"), and author thinks split bio can be resubmited and throttled again, which is wrong because split bio will continue to dispatch from caller.

Fixes: 9f5ede3c01f9 ("block: throttle split bio in case of iops limit") Cc: stable@vger.kernel.org Signed-off-by: Yu Kuai yukuai3@huawei.com Acked-by: Tejun Heo tj@kernel.org Link: https://lore.kernel.org/r/20220829022240.3348319-2-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe axboe@kernel.dk Conflicts: block/bio.c include/linux/bio.h [Context conflicts] Signed-off-by: Yu Kuai yukuai3@huawei.com --- block/bio.c | 2 -- block/blk-throttle.c | 20 ++++++-------------- block/blk-throttle.h | 2 +- include/linux/bio.h | 2 +- include/linux/blk_types.h | 2 +- 5 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/block/bio.c b/block/bio.c index 43bf0920aa32..e2eaa426ac38 100644 --- a/block/bio.c +++ b/block/bio.c @@ -683,8 +683,6 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src) bio->bi_disk = bio_src->bi_disk; bio->bi_partno = bio_src->bi_partno; bio_set_flag(bio, BIO_CLONED); - if (bio_flagged(bio_src, BIO_THROTTLED)) - bio_set_flag(bio, BIO_THROTTLED); bio->bi_opf = bio_src->bi_opf; bio->bi_ioprio = bio_src->bi_ioprio; bio->bi_write_hint = bio_src->bi_write_hint; diff --git a/block/blk-throttle.c b/block/blk-throttle.c index e7bdf36b8716..d4a12db068ce 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -841,7 +841,7 @@ static bool tg_within_bps_limit(struct throtl_grp *tg, struct bio *bio, unsigned int bio_size = throtl_bio_data_size(bio);

/* no need to throttle if this bio's bytes have been accounted */ - if (bps_limit == U64_MAX || bio_flagged(bio, BIO_THROTTLED)) { + if (bps_limit == U64_MAX || bio_flagged(bio, BIO_BPS_THROTTLED)) { if (wait) *wait = 0; return true; @@ -946,22 +946,13 @@ static void throtl_charge_bio(struct throtl_grp *tg, struct bio *bio) unsigned int bio_size = throtl_bio_data_size(bio);

/* Charge the bio to the group */ - if (!bio_flagged(bio, BIO_THROTTLED)) { + if (!bio_flagged(bio, BIO_BPS_THROTTLED)) { tg->bytes_disp[rw] += bio_size; tg->last_bytes_disp[rw] += bio_size; }

tg->io_disp[rw]++; tg->last_io_disp[rw]++; - - /* - * BIO_THROTTLED is used to prevent the same bio to be throttled - * more than once as a throttled bio will go through blk-throtl the - * second time when it eventually gets issued. Set it when a bio - * is being charged to a tg. - */ - if (!bio_flagged(bio, BIO_THROTTLED)) - bio_set_flag(bio, BIO_THROTTLED); }

/** @@ -1051,6 +1042,7 @@ static void tg_dispatch_one_bio(struct throtl_grp *tg, bool rw) sq->nr_queued[rw]--;

throtl_charge_bio(tg, bio); + bio_set_flag(bio, BIO_BPS_THROTTLED);

/* * If our parent is another tg, we just need to transfer @bio to @@ -2187,8 +2179,10 @@ bool __blk_throtl_bio(struct bio *bio) qn = &tg->qnode_on_parent[rw]; sq = sq->parent_sq; tg = sq_to_tg(sq); - if (!tg) + if (!tg) { + bio_set_flag(bio, BIO_BPS_THROTTLED); goto out_unlock; + } }

/* out-of-limit, queue to @tg */ @@ -2217,8 +2211,6 @@ bool __blk_throtl_bio(struct bio *bio) }

out_unlock: - bio_set_flag(bio, BIO_THROTTLED); - #ifdef CONFIG_BLK_DEV_THROTTLING_LOW if (throttled || !td->track_bio_latency) bio->bi_issue.value |= BIO_ISSUE_THROTL_SKIP_LATENCY; diff --git a/block/blk-throttle.h b/block/blk-throttle.h index b23a9f3abb82..438c50b3e071 100644 --- a/block/blk-throttle.h +++ b/block/blk-throttle.h @@ -172,7 +172,7 @@ static inline bool blk_throtl_bio(struct bio *bio) struct throtl_grp *tg = blkg_to_tg(bio->bi_blkg);

/* no need to throttle bps any more if the bio has been throttled */ - if (bio_flagged(bio, BIO_THROTTLED) && + if (bio_flagged(bio, BIO_BPS_THROTTLED) && !(tg->flags & THROTL_TG_HAS_IOPS_LIMIT)) return false;

diff --git a/include/linux/bio.h b/include/linux/bio.h index e895c84be0bc..680bf8f9d6e8 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -474,7 +474,7 @@ extern const char *bio_devname(struct bio *bio, char *buffer); #define bio_set_dev(bio, bdev) \ do { \ if ((bio)->bi_disk != (bdev)->bd_disk) \ - bio_clear_flag(bio, BIO_THROTTLED);\ + bio_clear_flag(bio, BIO_BPS_THROTTLED);\ (bio)->bi_disk = (bdev)->bd_disk; \ (bio)->bi_partno = (bdev)->bd_partno; \ bio_associate_blkg(bio); \ diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index b49d97547009..3298ac8ad281 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -300,7 +300,7 @@ enum { BIO_QUIET, /* Make BIO Quiet */ BIO_CHAIN, /* chained bio, ->bi_remaining in effect */ BIO_REFFED, /* bio has elevated ->bi_cnt */ - BIO_THROTTLED, /* This bio has already been subjected to + BIO_BPS_THROTTLED, /* This bio has already been subjected to * throttling rules. Don't do it again. */ BIO_TRACE_COMPLETION, /* bio_endio() should trace the final completion * of this bio. */

mainline inclusion from mainline-v6.1-rc1 commit 85496749904016f36b69332f73a1cf3ecfee828f category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

Currently, "tg->has_rules" and "tg->flags & THROTL_TG_HAS_IOPS_LIMIT" both try to bypass bios that don't need to be throttled, however, they are a little redundant and both not perfect:

1) "tg->has_rules" only distinguish read and write, but not iops and bps limit. 2) "tg->flags & THROTL_TG_HAS_IOPS_LIMIT" only check if iops limit exist, read and write is not distinguished, and bps limit is not checked.

tg->has_rules will extended to distinguish bps and iops in the following patch. There is no need to keep the flag.

Signed-off-by: Yu Kuai yukuai3@huawei.com Acked-by: Tejun Heo tj@kernel.org Link: https://lore.kernel.org/r/20220921095309.1481289-2-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe axboe@kernel.dk Conflicts: block/blk-throttle.h [commit ("8f9e7b65f833 block: cancel all throttled bios in del_gendisk()") is not backported.] Signed-off-by: Yu Kuai yukuai3@huawei.com --- block/blk-throttle.c | 16 ++-------------- block/blk-throttle.h | 6 ------ 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 3a5ae7998375..973359642481 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -435,24 +435,12 @@ static void tg_update_has_rules(struct throtl_grp *tg) struct throtl_grp *parent_tg = sq_to_tg(tg->service_queue.parent_sq); struct throtl_data *td = tg->td; int rw; - int has_iops_limit = 0; - - for (rw = READ; rw <= WRITE; rw++) { - unsigned int iops_limit = tg_iops_limit(tg, rw);

+ for (rw = READ; rw <= WRITE; rw++) tg->has_rules[rw] = (parent_tg && parent_tg->has_rules[rw]) || (td->limit_valid[td->limit_index] && (tg_bps_limit(tg, rw) != U64_MAX || - iops_limit != UINT_MAX)); - - if (iops_limit != UINT_MAX) - has_iops_limit = 1; - } - - if (has_iops_limit) - tg->flags |= THROTL_TG_HAS_IOPS_LIMIT; - else - tg->flags &= ~THROTL_TG_HAS_IOPS_LIMIT; + tg_iops_limit(tg, rw) != UINT_MAX)); }

static void throtl_pd_online(struct blkg_policy_data *pd) diff --git a/block/blk-throttle.h b/block/blk-throttle.h index f930dbf3ef3b..0ffdc5cb7222 100644 --- a/block/blk-throttle.h +++ b/block/blk-throttle.h @@ -55,7 +55,6 @@ struct throtl_service_queue { enum tg_state_flags { THROTL_TG_PENDING = 1 << 0, /* on parent's pending tree */ THROTL_TG_WAS_EMPTY = 1 << 1, /* bio_lists[] became non-empty */ - THROTL_TG_HAS_IOPS_LIMIT = 1 << 2, /* tg has iops limit */ };

enum { @@ -180,11 +179,6 @@ static inline bool blk_throtl_bio(struct bio *bio) { struct throtl_grp *tg = blkg_to_tg(bio->bi_blkg);

- /* no need to throttle bps any more if the bio has been throttled */ - if (bio_flagged(bio, BIO_BPS_THROTTLED) && - !(tg->flags & THROTL_TG_HAS_IOPS_LIMIT)) - return false; - if (!tg->has_rules[bio_data_dir(bio)]) return false;

From: Kemeng Shi shikemeng@huawei.com

mainline inclusion from mainline-v6.2-rc1 commit 84aca0a7e039c8735abc0f89f3f48e9006c0dfc7 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

Consider situation as following (on the default hierarchy): HDD | root (bps limit: 4k) | child (bps limit :8k) | fio bs=8k Rate of fio is supposed to be 4k, but result is 8k. Reason is as following: Size of single IO from fio is larger than bytes allowed in one throtl_slice in child, so IOs are always queued in child group first. When queued IOs in child are dispatched to parent group, BIO_BPS_THROTTLED is set and these IOs will not be limited by tg_within_bps_limit anymore. Fix this by only set BIO_BPS_THROTTLED when the bio traversed the entire tree.

There patch has no influence on situation which is not on the default hierarchy as each group is a single root group without parent.

Acked-by: Tejun Heo tj@kernel.org Signed-off-by: Kemeng Shi shikemeng@huawei.com Link: https://lore.kernel.org/r/20221205115709.251489-3-shikemeng@huaweicloud.com Signed-off-by: Jens Axboe axboe@kernel.dk Signed-off-by: Yu Kuai yukuai3@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 5e7580dde671..ba2f579a7934 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1079,7 +1079,6 @@ static void tg_dispatch_one_bio(struct throtl_grp *tg, bool rw) sq->nr_queued[rw]--;

throtl_charge_bio(tg, bio); - bio_set_flag(bio, BIO_BPS_THROTTLED);

/* * If our parent is another tg, we just need to transfer @bio to @@ -1092,6 +1091,7 @@ static void tg_dispatch_one_bio(struct throtl_grp *tg, bool rw) throtl_add_bio_tg(bio, &tg->qnode_on_parent[rw], parent_tg); start_parent_slice_with_credit(tg, parent_tg, rw); } else { + bio_set_flag(bio, BIO_BPS_THROTTLED); throtl_qnode_add_bio(bio, &tg->qnode_on_parent[rw], &parent_sq->queued[rw]); BUG_ON(tg->td->nr_queued[rw] <= 0);

From: Jinke Han hanjinke.666@bytedance.com

mainline inclusion from mainline-v6.5-rc1 commit ad7c3b41e86b59943a903d23c7b037d820e6270c category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

After commit f382fb0bcef4 ("block: remove legacy IO schedulers"), blkio.throttle.io_serviced and blkio.throttle.io_service_bytes become the only stable io stats interface of cgroup v1, and these statistics are done in the blk-throttle code. But the current code only counts the bios that are actually throttled. When the user does not add the throttle limit, the io stats for cgroup v1 has nothing. I fix it according to the statistical method of v2, and made it count all ios accurately.

Fixes: a7b36ee6ba29 ("block: move blk-throtl fast path inline") Tested-by: Andrea Righi andrea.righi@canonical.com Signed-off-by: Jinke Han hanjinke.666@bytedance.com Acked-by: Muchun Song songmuchun@bytedance.com Acked-by: Tejun Heo tj@kernel.org Link: https://lore.kernel.org/r/20230507170631.89607-1-hanjinke.666@bytedance.com Signed-off-by: Jens Axboe axboe@kernel.dk Conflicts: block/blk-cgroup.c [commit 0416f3be58c6 ("blk-cgroup: don't update io stat for root cgroup") is not backported.] Signed-off-by: Yu Kuai yukuai3@huawei.com --- block/blk-cgroup.c | 6 ++++-- block/blk-throttle.c | 6 ------ block/blk-throttle.h | 9 +++++++++ 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 0872b392360d..0805864543be 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1953,6 +1953,9 @@ void blk_cgroup_bio_start(struct bio *bio) struct blkg_iostat_set *bis; unsigned long flags;

+ if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) + return; + cpu = get_cpu(); bis = per_cpu_ptr(bio->bi_blkg->iostat_cpu, cpu); flags = u64_stats_update_begin_irqsave(&bis->sync); @@ -1968,8 +1971,7 @@ void blk_cgroup_bio_start(struct bio *bio) bis->cur.ios[rwd]++;

u64_stats_update_end_irqrestore(&bis->sync, flags); - if (cgroup_subsys_on_dfl(io_cgrp_subsys)) - cgroup_rstat_updated(bio->bi_blkg->blkcg->css.cgroup, cpu); + cgroup_rstat_updated(bio->bi_blkg->blkcg->css.cgroup, cpu); put_cpu(); }

diff --git a/block/blk-throttle.c b/block/blk-throttle.c index d04a72b30faf..79ffce4bc213 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -2149,12 +2149,6 @@ bool __blk_throtl_bio(struct bio *bio)

rcu_read_lock();

- if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) { - blkg_rwstat_add(&tg->stat_bytes, bio->bi_opf, - bio->bi_iter.bi_size); - blkg_rwstat_add(&tg->stat_ios, bio->bi_opf, 1); - } - spin_lock_irq(&q->queue_lock);

throtl_update_latency_buckets(td); diff --git a/block/blk-throttle.h b/block/blk-throttle.h index 45fcf7aa0eb5..cb7f7d0e6f0d 100644 --- a/block/blk-throttle.h +++ b/block/blk-throttle.h @@ -182,6 +182,15 @@ static inline bool blk_should_throtl(struct bio *bio) struct throtl_grp *tg = blkg_to_tg(bio->bi_blkg); int rw = bio_data_dir(bio);

+ if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) { + if (!bio_flagged(bio, BIO_CGROUP_ACCT)) { + bio_set_flag(bio, BIO_CGROUP_ACCT); + blkg_rwstat_add(&tg->stat_bytes, bio->bi_opf, + bio->bi_iter.bi_size); + } + blkg_rwstat_add(&tg->stat_ios, bio->bi_opf, 1); + } + /* iops limit is always counted */ if (tg->has_rules_iops[rw]) return true;

mainline inclusion from mainline-v6.6-rc1 commit bb8d5587bdc3ab211e1eae2eeb966f7a7d1f9c0b category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

carryover_ios/bytes[] can be negative in the case that ios are dispatched in the slice in advance, and then configuration is updated. For example:

1) set iops limit to 1000, and slice start is 0, slice end is 100ms; 2) current time is 0, and 100 ios are dispatched, those ios will not be throttled, hence io_disp is 100; 3) still at current time 0, update iops limit to 100, then carryover_ios is (0 - 100) = -100; 4) then, dispatch a new io at time 0, the expected result is that this io will wait for 1s. The calculation in tg_within_iops_limit:

io_disp = 0; io_allowed = calculate_io_allowed + carryover_ios = 10 + (-100) = -90; io won't be throttled if (io_disp + 1 < io_allowed) passed.

Before this patch, in step 4) (io_disp + 1 < io_allowed) is passed, because -90 for unsigned value is very huge, and such io won't be throttled.

Fix this problem by checking if 'io/bytes_allowed' is negative first.

Signed-off-by: Yu Kuai yukuai3@huawei.com Acked-by: Tejun Heo tj@kernel.org Link: https://lore.kernel.org/r/20230816012708.1193747-3-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe axboe@kernel.dk --- block/blk-throttle.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c index fe3ee1a90aaf..91a22d49f5c3 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -839,7 +839,7 @@ static unsigned long tg_within_iops_limit(struct throtl_grp *tg, struct bio *bio u32 iops_limit) { bool rw = bio_data_dir(bio); - unsigned int io_allowed; + int io_allowed; unsigned long jiffy_elapsed, jiffy_wait, jiffy_elapsed_rnd;

if (iops_limit == UINT_MAX) { @@ -852,9 +852,8 @@ static unsigned long tg_within_iops_limit(struct throtl_grp *tg, struct bio *bio jiffy_elapsed_rnd = roundup(jiffy_elapsed + 1, tg->td->throtl_slice); io_allowed = calculate_io_allowed(iops_limit, jiffy_elapsed_rnd) + tg->carryover_ios[rw]; - if (tg->io_disp[rw] + 1 <= io_allowed) { + if (io_allowed > 0 && tg->io_disp[rw] + 1 <= io_allowed) return 0; - }

/* Calc approx time to dispatch */ jiffy_wait = jiffy_elapsed_rnd - jiffy_elapsed; @@ -865,7 +864,8 @@ static unsigned long tg_within_bps_limit(struct throtl_grp *tg, struct bio *bio, u64 bps_limit) { bool rw = bio_data_dir(bio); - u64 bytes_allowed, extra_bytes; + long long bytes_allowed; + u64 extra_bytes; unsigned long jiffy_elapsed, jiffy_wait, jiffy_elapsed_rnd; unsigned int bio_size = throtl_bio_data_size(bio);

@@ -883,9 +883,8 @@ static unsigned long tg_within_bps_limit(struct throtl_grp *tg, struct bio *bio, jiffy_elapsed_rnd = roundup(jiffy_elapsed_rnd, tg->td->throtl_slice); bytes_allowed = calculate_bytes_allowed(bps_limit, jiffy_elapsed_rnd) + tg->carryover_bytes[rw]; - if (tg->bytes_disp[rw] + bio_size <= bytes_allowed) { + if (bytes_allowed > 0 && tg->bytes_disp[rw] + bio_size <= bytes_allowed) return 0; - }

/* Calc approx time to dispatch */ extra_bytes = tg->bytes_disp[rw] + bio_size - bytes_allowed;

mainline inclusion from mainline-v6.12-rc1 commit 29390bb5661d49d10424ad8e915230de1f7074c9 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IAUKH4 CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

Currently, blk-throttle handle all IO fifo, hence if data IO is throttled and then meta IO is dispatched, the meta IO will have to wait for the data IO, causing priority inversion problems.

This patch support to handle metadata first and then pay debt while throttling data.

Test script: use cgroup v1 to throttle root cgroup, then create new dir and file while write back is throttled

test() { mkdir /mnt/test/xxx touch /mnt/test/xxx/1 sync /mnt/test/xxx sync /mnt/test/xxx }

mkfs.ext4 -F /dev/nvme0n1 -E lazy_itable_init=0,lazy_journal_init=0 mount /dev/nvme0n1 /mnt/test

echo "259:0 $((1024*1024))" > /sys/fs/cgroup/blkio/blkio.throttle.write_bps_device dd if=/dev/zero of=/mnt/test/foo1 bs=16M count=1 conv=fdatasync status=none & sleep 4

time test echo "259:0 0" > /sys/fs/cgroup/blkio/blkio.throttle.write_bps_device

sleep 1 umount /dev/nvme0n1

Test result: time cost for creating new dir and file before this patch: 14s after this patch: 0.1s

Signed-off-by: Yu Kuai yukuai3@huawei.com Acked-by: Tejun Heo tj@kernel.org Link: https://lore.kernel.org/r/20240903135149.271857-3-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe axboe@kernel.dk Conflicts: block/blk-throttle.c [commit bf20ab538c81 ("blk-throttle: remove CONFIG_BLK_DEV_THROTTLING_LOW") is not backported.] Signed-off-by: Yu Kuai yukuai3@huawei.com --- block/blk-throttle.c | 63 +++++++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 21 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c index f9aeaa2ce9e7..5e299ea26d85 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -2140,6 +2140,22 @@ static inline void throtl_update_latency_buckets(struct throtl_data *td) } #endif

+static bool tg_within_limit(struct throtl_grp *tg, struct bio *bio, bool rw) +{ + /* throtl is FIFO - if bios are already queued, should queue */ + if (tg->service_queue.nr_queued[rw]) + return false; + + return tg_may_dispatch(tg, bio, NULL); +} + +static void tg_dispatch_in_debt(struct throtl_grp *tg, struct bio *bio, bool rw) +{ + if (!bio_flagged(bio, BIO_BPS_THROTTLED)) + tg->carryover_bytes[rw] -= throtl_bio_data_size(bio); + tg->carryover_ios[rw]--; +} + bool __blk_throtl_bio(struct bio *bio) { struct request_queue *q = bio->bi_disk->queue; @@ -2167,12 +2183,33 @@ bool __blk_throtl_bio(struct bio *bio) tg->last_low_overflow_time[rw] = jiffies; throtl_downgrade_check(tg); throtl_upgrade_check(tg); - /* throtl is FIFO - if bios are already queued, should queue */ - if (sq->nr_queued[rw]) - break;

- /* if above limits, break to queue */ - if (!tg_may_dispatch(tg, bio, NULL)) { + if (tg_within_limit(tg, bio, rw)) { + /* within limits, let's charge and dispatch directly */ + throtl_charge_bio(tg, bio); + + /* + * We need to trim slice even when bios are not being + * queued otherwise it might happen that a bio is not + * queued for a long time and slice keeps on extending + * and trim is not called for a long time. Now if limits + * are reduced suddenly we take into account all the IO + * dispatched so far at new low rate and * newly queued + * IO gets a really long dispatch time. + * + * So keep on trimming slice even if bio is not queued. + */ + throtl_trim_slice(tg, rw); + } else if (bio_issue_as_root_blkg(bio)) { + /* + * IOs which may cause priority inversions are + * dispatched directly, even if they're over limit. + * Debts are handled by carryover_bytes/ios while + * calculating wait time. + */ + tg_dispatch_in_debt(tg, bio, rw); + } else { + /* if above limits, break to queue */ tg->last_low_overflow_time[rw] = jiffies; if (throtl_can_upgrade(td, tg)) { throtl_upgrade_state(td); @@ -2181,22 +2218,6 @@ bool __blk_throtl_bio(struct bio *bio) break; }

- /* within limits, let's charge and dispatch directly */ - throtl_charge_bio(tg, bio); - - /* - * We need to trim slice even when bios are not being queued - * otherwise it might happen that a bio is not queued for - * a long time and slice keeps on extending and trim is not - * called for a long time. Now if limits are reduced suddenly - * we take into account all the IO dispatched so far at new - * low rate and * newly queued IO gets a really long dispatch - * time. - * - * So keep on trimming slice even if bio is not queued. - */ - throtl_trim_slice(tg, rw); - /* * @bio passed through this layer without being throttled. * Climb up the ladder. If we're already at the top, it