* [PATCH v2 0/8] Some improvements for blk throttle
@ 2020-10-08 3:52 Baolin Wang
2020-10-08 3:52 ` [PATCH v2 1/8] blk-throttle: Remove a meaningless parameter for throtl_downgrade_state() Baolin Wang
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: Baolin Wang @ 2020-10-08 3:52 UTC (permalink / raw)
To: tj, axboe; +Cc: baolin.wang, baolin.wang7, linux-block, cgroups, linux-kernel
Hi,
This patch set did some improvements for blk throttle, please
help to review. Thanks.
Changes from v1:
- Add another 4 new patches in this patch set.
Baolin Wang (8):
blk-throttle: Remove a meaningless parameter for
throtl_downgrade_state()
blk-throttle: Avoid getting the current time if tg->last_finish_time
is 0
blk-throttle: Avoid tracking latency if low limit is invalid
blk-throttle: Fix IO hang for a corner case
blk-throttle: Move the list operation after list validation
blk-throttle: Move service tree validation out of the
throtl_rb_first()
blk-throttle: Open code __throtl_de/enqueue_tg()
blk-throttle: Re-use the throtl_set_slice_end()
block/blk-throttle.c | 69 ++++++++++++++++++++++++++--------------------------
1 file changed, 35 insertions(+), 34 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/8] blk-throttle: Remove a meaningless parameter for throtl_downgrade_state()
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
@ 2020-10-08 3:52 ` Baolin Wang
2020-10-08 3:52 ` [PATCH v2 2/8] blk-throttle: Avoid getting the current time if tg->last_finish_time is 0 Baolin Wang
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-10-08 3:52 UTC (permalink / raw)
To: tj, axboe; +Cc: baolin.wang, baolin.wang7, linux-block, cgroups, linux-kernel
The throtl_downgrade_state() is always used to change to LIMIT_LOW
limitation, thus remove the latter meaningless parameter which
indicates the limitation index.
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
block/blk-throttle.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 36ba61c..4007b26 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1970,7 +1970,7 @@ static void throtl_upgrade_state(struct throtl_data *td)
queue_work(kthrotld_workqueue, &td->dispatch_work);
}
-static void throtl_downgrade_state(struct throtl_data *td, int new)
+static void throtl_downgrade_state(struct throtl_data *td)
{
td->scale /= 2;
@@ -1980,7 +1980,7 @@ static void throtl_downgrade_state(struct throtl_data *td, int new)
return;
}
- td->limit_index = new;
+ td->limit_index = LIMIT_LOW;
td->low_downgrade_time = jiffies;
}
@@ -2067,7 +2067,7 @@ static void throtl_downgrade_check(struct throtl_grp *tg)
* cgroups
*/
if (throtl_hierarchy_can_downgrade(tg))
- throtl_downgrade_state(tg->td, LIMIT_LOW);
+ throtl_downgrade_state(tg->td);
tg->last_bytes_disp[READ] = 0;
tg->last_bytes_disp[WRITE] = 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/8] blk-throttle: Avoid getting the current time if tg->last_finish_time is 0
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
2020-10-08 3:52 ` [PATCH v2 1/8] blk-throttle: Remove a meaningless parameter for throtl_downgrade_state() Baolin Wang
@ 2020-10-08 3:52 ` Baolin Wang
2020-10-08 3:52 ` [PATCH v2 3/8] blk-throttle: Avoid tracking latency if low limit is invalid Baolin Wang
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-10-08 3:52 UTC (permalink / raw)
To: tj, axboe; +Cc: baolin.wang, baolin.wang7, linux-block, cgroups, linux-kernel
We only update the tg->last_finish_time when the low limitaion is
enabled, so we can move the tg->last_finish_time validation a little
forward to avoid getting the unnecessary current time stamp if the
the low limitation is not enabled.
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
block/blk-throttle.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 4007b26..7e72102 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -2077,10 +2077,14 @@ static void throtl_downgrade_check(struct throtl_grp *tg)
static void blk_throtl_update_idletime(struct throtl_grp *tg)
{
- unsigned long now = ktime_get_ns() >> 10;
+ unsigned long now;
unsigned long last_finish_time = tg->last_finish_time;
- if (now <= last_finish_time || last_finish_time == 0 ||
+ if (last_finish_time == 0)
+ return;
+
+ now = ktime_get_ns() >> 10;
+ if (now <= last_finish_time ||
last_finish_time == tg->checked_last_finish_time)
return;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/8] blk-throttle: Avoid tracking latency if low limit is invalid
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
2020-10-08 3:52 ` [PATCH v2 1/8] blk-throttle: Remove a meaningless parameter for throtl_downgrade_state() Baolin Wang
2020-10-08 3:52 ` [PATCH v2 2/8] blk-throttle: Avoid getting the current time if tg->last_finish_time is 0 Baolin Wang
@ 2020-10-08 3:52 ` Baolin Wang
2020-10-08 3:52 ` [PATCH v2 4/8] blk-throttle: Fix IO hang for a corner case Baolin Wang
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-10-08 3:52 UTC (permalink / raw)
To: tj, axboe; +Cc: baolin.wang, baolin.wang7, linux-block, cgroups, linux-kernel
The IO latency tracking is only for LOW limit, so we should add a
validation to avoid redundant latency tracking if the LOW limit
is not valid.
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
block/blk-throttle.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 7e72102..b0d8f7c 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -2100,7 +2100,7 @@ static void throtl_update_latency_buckets(struct throtl_data *td)
unsigned long last_latency[2] = { 0 };
unsigned long latency[2];
- if (!blk_queue_nonrot(td->queue))
+ if (!blk_queue_nonrot(td->queue) || !td->limit_valid[LIMIT_LOW])
return;
if (time_before(jiffies, td->last_calculate_time + HZ))
return;
@@ -2338,6 +2338,8 @@ void blk_throtl_bio_endio(struct bio *bio)
if (!blkg)
return;
tg = blkg_to_tg(blkg);
+ if (!tg->td->limit_valid[LIMIT_LOW])
+ return;
finish_time_ns = ktime_get_ns();
tg->last_finish_time = finish_time_ns >> 10;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/8] blk-throttle: Fix IO hang for a corner case
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
` (2 preceding siblings ...)
2020-10-08 3:52 ` [PATCH v2 3/8] blk-throttle: Avoid tracking latency if low limit is invalid Baolin Wang
@ 2020-10-08 3:52 ` Baolin Wang
2020-10-08 3:52 ` [PATCH v2 5/8] blk-throttle: Move the list operation after list validation Baolin Wang
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-10-08 3:52 UTC (permalink / raw)
To: tj, axboe; +Cc: baolin.wang, baolin.wang7, linux-block, cgroups, linux-kernel
It can not scale up in throtl_adjusted_limit() if we set bps or iops is
1, which will cause IO hang when enable low limit. Thus we should treat
1 as a illegal value to avoid this issue.
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
block/blk-throttle.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index b0d8f7c..0649bce 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1687,13 +1687,13 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of,
goto out_finish;
ret = -EINVAL;
- if (!strcmp(tok, "rbps"))
+ if (!strcmp(tok, "rbps") && val > 1)
v[0] = val;
- else if (!strcmp(tok, "wbps"))
+ else if (!strcmp(tok, "wbps") && val > 1)
v[1] = val;
- else if (!strcmp(tok, "riops"))
+ else if (!strcmp(tok, "riops") && val > 1)
v[2] = min_t(u64, val, UINT_MAX);
- else if (!strcmp(tok, "wiops"))
+ else if (!strcmp(tok, "wiops") && val > 1)
v[3] = min_t(u64, val, UINT_MAX);
else if (off == LIMIT_LOW && !strcmp(tok, "idle"))
idle_time = val;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 5/8] blk-throttle: Move the list operation after list validation
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
` (3 preceding siblings ...)
2020-10-08 3:52 ` [PATCH v2 4/8] blk-throttle: Fix IO hang for a corner case Baolin Wang
@ 2020-10-08 3:52 ` Baolin Wang
2020-10-08 3:52 ` [PATCH v2 6/8] blk-throttle: Move service tree validation out of the throtl_rb_first() Baolin Wang
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-10-08 3:52 UTC (permalink / raw)
To: tj, axboe; +Cc: baolin.wang, baolin.wang7, linux-block, cgroups, linux-kernel
We should move the list operation after validation.
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
block/blk-throttle.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 0649bce..f1bcb5c 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -423,12 +423,13 @@ static void throtl_qnode_add_bio(struct bio *bio, struct throtl_qnode *qn,
*/
static struct bio *throtl_peek_queued(struct list_head *queued)
{
- struct throtl_qnode *qn = list_first_entry(queued, struct throtl_qnode, node);
+ struct throtl_qnode *qn;
struct bio *bio;
if (list_empty(queued))
return NULL;
+ qn = list_first_entry(queued, struct throtl_qnode, node);
bio = bio_list_peek(&qn->bios);
WARN_ON_ONCE(!bio);
return bio;
@@ -451,12 +452,13 @@ static struct bio *throtl_peek_queued(struct list_head *queued)
static struct bio *throtl_pop_queued(struct list_head *queued,
struct throtl_grp **tg_to_put)
{
- struct throtl_qnode *qn = list_first_entry(queued, struct throtl_qnode, node);
+ struct throtl_qnode *qn;
struct bio *bio;
if (list_empty(queued))
return NULL;
+ qn = list_first_entry(queued, struct throtl_qnode, node);
bio = bio_list_pop(&qn->bios);
WARN_ON_ONCE(!bio);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 6/8] blk-throttle: Move service tree validation out of the throtl_rb_first()
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
` (4 preceding siblings ...)
2020-10-08 3:52 ` [PATCH v2 5/8] blk-throttle: Move the list operation after list validation Baolin Wang
@ 2020-10-08 3:52 ` Baolin Wang
2020-10-08 3:52 ` [PATCH v2 7/8] blk-throttle: Open code __throtl_de/enqueue_tg() Baolin Wang
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-10-08 3:52 UTC (permalink / raw)
To: tj, axboe; +Cc: baolin.wang, baolin.wang7, linux-block, cgroups, linux-kernel
The throtl_schedule_next_dispatch() will validate if the service queue
is empty before calling update_min_dispatch_time(), and the
update_min_dispatch_time() will call throtl_rb_first(), which will
validate service queue again.
Thus we can move the service queue validation out of the
throtl_rb_first() to remove the redundant validation in the fast path.
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
block/blk-throttle.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index f1bcb5c..38aed8b 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -638,9 +638,6 @@ static void throtl_pd_free(struct blkg_policy_data *pd)
throtl_rb_first(struct throtl_service_queue *parent_sq)
{
struct rb_node *n;
- /* Service tree is empty */
- if (!parent_sq->nr_pending)
- return NULL;
n = rb_first_cached(&parent_sq->pending_tree);
WARN_ON_ONCE(!n);
@@ -1224,9 +1221,13 @@ static int throtl_select_dispatch(struct throtl_service_queue *parent_sq)
unsigned int nr_disp = 0;
while (1) {
- struct throtl_grp *tg = throtl_rb_first(parent_sq);
+ struct throtl_grp *tg;
struct throtl_service_queue *sq;
+ if (!parent_sq->nr_pending)
+ break;
+
+ tg = throtl_rb_first(parent_sq);
if (!tg)
break;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 7/8] blk-throttle: Open code __throtl_de/enqueue_tg()
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
` (5 preceding siblings ...)
2020-10-08 3:52 ` [PATCH v2 6/8] blk-throttle: Move service tree validation out of the throtl_rb_first() Baolin Wang
@ 2020-10-08 3:52 ` Baolin Wang
2020-10-08 3:52 ` [PATCH v2 8/8] blk-throttle: Re-use the throtl_set_slice_end() Baolin Wang
2020-10-08 14:02 ` [PATCH v2 0/8] Some improvements for blk throttle Jens Axboe
8 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-10-08 3:52 UTC (permalink / raw)
To: tj, axboe; +Cc: baolin.wang, baolin.wang7, linux-block, cgroups, linux-kernel
The __throtl_de/enqueue_tg() functions are only be called by
throtl_de/enqueue_tg(), thus we can just open code them to
make code more readable.
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
block/blk-throttle.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 38aed8b..fc5c14f 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -691,29 +691,21 @@ static void tg_service_queue_add(struct throtl_grp *tg)
leftmost);
}
-static void __throtl_enqueue_tg(struct throtl_grp *tg)
-{
- tg_service_queue_add(tg);
- tg->flags |= THROTL_TG_PENDING;
- tg->service_queue.parent_sq->nr_pending++;
-}
-
static void throtl_enqueue_tg(struct throtl_grp *tg)
{
- if (!(tg->flags & THROTL_TG_PENDING))
- __throtl_enqueue_tg(tg);
-}
-
-static void __throtl_dequeue_tg(struct throtl_grp *tg)
-{
- throtl_rb_erase(&tg->rb_node, tg->service_queue.parent_sq);
- tg->flags &= ~THROTL_TG_PENDING;
+ if (!(tg->flags & THROTL_TG_PENDING)) {
+ tg_service_queue_add(tg);
+ tg->flags |= THROTL_TG_PENDING;
+ tg->service_queue.parent_sq->nr_pending++;
+ }
}
static void throtl_dequeue_tg(struct throtl_grp *tg)
{
- if (tg->flags & THROTL_TG_PENDING)
- __throtl_dequeue_tg(tg);
+ if (tg->flags & THROTL_TG_PENDING) {
+ throtl_rb_erase(&tg->rb_node, tg->service_queue.parent_sq);
+ tg->flags &= ~THROTL_TG_PENDING;
+ }
}
/* Call with queue lock held */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 8/8] blk-throttle: Re-use the throtl_set_slice_end()
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
` (6 preceding siblings ...)
2020-10-08 3:52 ` [PATCH v2 7/8] blk-throttle: Open code __throtl_de/enqueue_tg() Baolin Wang
@ 2020-10-08 3:52 ` Baolin Wang
2020-10-08 14:02 ` [PATCH v2 0/8] Some improvements for blk throttle Jens Axboe
8 siblings, 0 replies; 10+ messages in thread
From: Baolin Wang @ 2020-10-08 3:52 UTC (permalink / raw)
To: tj, axboe; +Cc: baolin.wang, baolin.wang7, linux-block, cgroups, linux-kernel
Re-use throtl_set_slice_end() to remove duplicate code.
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.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 fc5c14f..b771c42 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -808,7 +808,7 @@ static inline void throtl_set_slice_end(struct throtl_grp *tg, bool rw,
static inline void throtl_extend_slice(struct throtl_grp *tg, bool rw,
unsigned long jiffy_end)
{
- tg->slice_end[rw] = roundup(jiffy_end, tg->td->throtl_slice);
+ throtl_set_slice_end(tg, rw, jiffy_end);
throtl_log(&tg->service_queue,
"[%c] extend slice start=%lu end=%lu jiffies=%lu",
rw == READ ? 'R' : 'W', tg->slice_start[rw],
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/8] Some improvements for blk throttle
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
` (7 preceding siblings ...)
2020-10-08 3:52 ` [PATCH v2 8/8] blk-throttle: Re-use the throtl_set_slice_end() Baolin Wang
@ 2020-10-08 14:02 ` Jens Axboe
8 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2020-10-08 14:02 UTC (permalink / raw)
To: Baolin Wang, tj; +Cc: baolin.wang7, linux-block, cgroups, linux-kernel
On 10/7/20 9:52 PM, Baolin Wang wrote:
> Hi,
>
> This patch set did some improvements for blk throttle, please
> help to review. Thanks.
>
> Changes from v1:
> - Add another 4 new patches in this patch set.
>
> Baolin Wang (8):
> blk-throttle: Remove a meaningless parameter for
> throtl_downgrade_state()
> blk-throttle: Avoid getting the current time if tg->last_finish_time
> is 0
> blk-throttle: Avoid tracking latency if low limit is invalid
> blk-throttle: Fix IO hang for a corner case
> blk-throttle: Move the list operation after list validation
> blk-throttle: Move service tree validation out of the
> throtl_rb_first()
> blk-throttle: Open code __throtl_de/enqueue_tg()
> blk-throttle: Re-use the throtl_set_slice_end()
>
> block/blk-throttle.c | 69 ++++++++++++++++++++++++++--------------------------
> 1 file changed, 35 insertions(+), 34 deletions(-)
LGTM, applied, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-08 14:02 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08 3:52 [PATCH v2 0/8] Some improvements for blk throttle Baolin Wang
2020-10-08 3:52 ` [PATCH v2 1/8] blk-throttle: Remove a meaningless parameter for throtl_downgrade_state() Baolin Wang
2020-10-08 3:52 ` [PATCH v2 2/8] blk-throttle: Avoid getting the current time if tg->last_finish_time is 0 Baolin Wang
2020-10-08 3:52 ` [PATCH v2 3/8] blk-throttle: Avoid tracking latency if low limit is invalid Baolin Wang
2020-10-08 3:52 ` [PATCH v2 4/8] blk-throttle: Fix IO hang for a corner case Baolin Wang
2020-10-08 3:52 ` [PATCH v2 5/8] blk-throttle: Move the list operation after list validation Baolin Wang
2020-10-08 3:52 ` [PATCH v2 6/8] blk-throttle: Move service tree validation out of the throtl_rb_first() Baolin Wang
2020-10-08 3:52 ` [PATCH v2 7/8] blk-throttle: Open code __throtl_de/enqueue_tg() Baolin Wang
2020-10-08 3:52 ` [PATCH v2 8/8] blk-throttle: Re-use the throtl_set_slice_end() Baolin Wang
2020-10-08 14:02 ` [PATCH v2 0/8] Some improvements for blk throttle Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).