| Message ID | 20221123060401.20392-6-shikemeng@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | A few bugfix and cleanup patches for blk-throttle | expand |
On Wed, Nov 23, 2022 at 02:03:55PM +0800, Kemeng Shi wrote: > -static bool throtl_tg_can_upgrade(struct throtl_grp *tg) > +static bool throtl_tg_reach_low_limit(struct throtl_grp *tg, int rw) > { > struct throtl_service_queue *sq = &tg->service_queue; > - bool read_limit, write_limit; > + bool limit = tg->bps[rw][LIMIT_LOW] || tg->iops[rw][LIMIT_LOW]; > > /* > * if cgroup reaches low limit (if low limit is 0, the cgroup always > * reaches), it's ok to upgrade to next limit > */ > - read_limit = tg->bps[READ][LIMIT_LOW] || tg->iops[READ][LIMIT_LOW]; > - write_limit = tg->bps[WRITE][LIMIT_LOW] || tg->iops[WRITE][LIMIT_LOW]; > - if (!read_limit && !write_limit) > - return true; > - if (read_limit && sq->nr_queued[READ] && > - (!write_limit || sq->nr_queued[WRITE])) > - return true; > - if (write_limit && sq->nr_queued[WRITE] && > - (!read_limit || sq->nr_queued[READ])) > + return !limit || sq->nr_queued[rw]. > +} > + > +static bool throtl_tg_can_upgrade(struct throtl_grp *tg) > +{ > + if (throtl_tg_reach_low_limit(tg, READ) && > + throtl_tg_reach_low_limit(tg, WRITE)) Are the conditions being checked actually equivalent? If so, can you explicitly explain that these are equivalent conditions? If not, what are we changing exactly? Thanks.
on 11/24/2022 2:26 AM, Tejun Heo wrote: > On Wed, Nov 23, 2022 at 02:03:55PM +0800, Kemeng Shi wrote: >> -static bool throtl_tg_can_upgrade(struct throtl_grp *tg) >> +static bool throtl_tg_reach_low_limit(struct throtl_grp *tg, int rw) >> { >> struct throtl_service_queue *sq = &tg->service_queue; >> - bool read_limit, write_limit; >> + bool limit = tg->bps[rw][LIMIT_LOW] || tg->iops[rw][LIMIT_LOW]; >> >> /* >> * if cgroup reaches low limit (if low limit is 0, the cgroup always >> * reaches), it's ok to upgrade to next limit >> */ >> - read_limit = tg->bps[READ][LIMIT_LOW] || tg->iops[READ][LIMIT_LOW]; >> - write_limit = tg->bps[WRITE][LIMIT_LOW] || tg->iops[WRITE][LIMIT_LOW]; >> - if (!read_limit && !write_limit) >> - return true; >> - if (read_limit && sq->nr_queued[READ] && >> - (!write_limit || sq->nr_queued[WRITE])) >> - return true; >> - if (write_limit && sq->nr_queued[WRITE] && >> - (!read_limit || sq->nr_queued[READ])) >> + return !limit || sq->nr_queued[rw]. >> +} >> + >> +static bool throtl_tg_can_upgrade(struct throtl_grp *tg) >> +{ >> + if (throtl_tg_reach_low_limit(tg, READ) && >> + throtl_tg_reach_low_limit(tg, WRITE)) > > Are the conditions being checked actually equivalent? If so, can you > explicitly explain that these are equivalent conditions? If not, what are we > changing exactly?All replaced conditions to return true are as following: condition 1 (!read_limit && !write_limit) condition 2 read_limit && sq->nr_queued[READ] && (!write_limit || sq->nr_queued[WRITE]) condition 3 write_limit && sq->nr_queued[WRITE] && (!read_limit || sq->nr_queued[READ]) Transfering condition 2 as following: read_limit && sq->nr_queued[READ] && (!write_limit || sq->nr_queued[WRITE]) is equivalent to (read_limit && sq->nr_queued[READ]) && (!write_limit || (write_limit && sq->nr_queued[WRITE])) is equivalent to (read_limit && sq->nr_queued[READ] && !write_limit) || ((read_limit && sq->nr_queued[READ] && (write_limit && sq->nr_queued[WRITE])) Transfering condition 3 as following: write_limit && sq->nr_queued[WRITE] && (!read_limit || sq->nr_queued[READ]) is equivalent to (write_limit && sq->nr_queued[WRITE]) && (!read_limit || (read_limit && sq->nr_queued[READ])) is equivalent to ((write_limit && sq->nr_queued[WRITE]) && !read_limit) || ((write_limit && sq->nr_queued[WRITE]) && (read_limit && sq->nr_queued[READ])) All replaced conditions to return true are collected as folloing: condition 1.1 (!read_limit && !write_limit) condition 1.2 (read_limit && sq->nr_queued[READ] && !write_limit) condition 1.3 ((read_limit && sq->nr_queued[READ] && (write_limit && sq->nr_queued[WRITE])) condition 1.4 (write_limit && sq->nr_queued[WRITE]) && !read_limit) condition 1.5 (the same as 1.3, can be ingored) (write_limit && sq->nr_queued[WRITE]) && (read_limit && sq->nr_queued[READ])) Condtions to return true in this patch is: (!read_limit || (read_limit && sq->nr_queued[READ])) && (!write_limit || (write_limit && sq->nr_queued[WRITE])) As "(a || b) && (c || d)" can be extracted to (a && c) or (a && d) or (b && c) or (b && d ). So we can extract condtions to condition 2.1 !read_limit && !write_limit condition 2.2 !read_limit && (write_limit && sq->nr_queued[WRITE]) condition 2.3 (read_limit && sq->nr_queued[READ]) && !write_limit condition 2.4 (read_limit && sq->nr_queued[READ]) && (write_limit && sq->nr_queued[WRITE]) Conditions match as following: condition 1.1 = condition 2.1 condition 1.2 = condition 2.3 condition 1.3 = condition 2.4 condition 1.4 = condition 2.2
diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 01e30380c19b..322a6ee38fb6 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1800,24 +1800,22 @@ static bool throtl_tg_is_idle(struct throtl_grp *tg) return ret; } -static bool throtl_tg_can_upgrade(struct throtl_grp *tg) +static bool throtl_tg_reach_low_limit(struct throtl_grp *tg, int rw) { struct throtl_service_queue *sq = &tg->service_queue; - bool read_limit, write_limit; + bool limit = tg->bps[rw][LIMIT_LOW] || tg->iops[rw][LIMIT_LOW]; /* * if cgroup reaches low limit (if low limit is 0, the cgroup always * reaches), it's ok to upgrade to next limit */ - read_limit = tg->bps[READ][LIMIT_LOW] || tg->iops[READ][LIMIT_LOW]; - write_limit = tg->bps[WRITE][LIMIT_LOW] || tg->iops[WRITE][LIMIT_LOW]; - if (!read_limit && !write_limit) - return true; - if (read_limit && sq->nr_queued[READ] && - (!write_limit || sq->nr_queued[WRITE])) - return true; - if (write_limit && sq->nr_queued[WRITE] && - (!read_limit || sq->nr_queued[READ])) + return !limit || sq->nr_queued[rw]. +} + +static bool throtl_tg_can_upgrade(struct throtl_grp *tg) +{ + if (throtl_tg_reach_low_limit(tg, READ) && + throtl_tg_reach_low_limit(tg, WRITE)) return true; if (time_after_eq(jiffies,
Cgroup reaches low limit if limit is zero or io is already queued. Cgroup will pass upgrade check if low limits of READ and WRITE are both reached. Simpfy the check code described above to removce repeat check and improve readability. There is no functional change. Signed-off-by: Kemeng Shi <shikemeng@huawei.com> --- block/blk-throttle.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)