diff mbox series

[v7,8/9] blk-throttle: cleanup tg_update_disptime()

Message ID 20220802140415.2960284-9-yukuai1@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series bugfix and cleanup for blk-throttle | expand

Commit Message

Yu Kuai Aug. 2, 2022, 2:04 p.m. UTC 
From: Yu Kuai <yukuai3@huawei.com>

tg_update_disptime() only need to adjust postion for 'tg' in
'parent_sq', there is no need to call throtl_enqueue/dequeue_tg().

Save a little overhead in tg_update_disptime() and prepare to cleanup
flag 'THROTL_TG_PENDING', there are no functional changes.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/blk-throttle.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Tejun Heo Aug. 16, 2022, 8:09 p.m. UTC | #1 
On Tue, Aug 02, 2022 at 10:04:14PM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> tg_update_disptime() only need to adjust postion for 'tg' in
> 'parent_sq', there is no need to call throtl_enqueue/dequeue_tg().
> 
> Save a little overhead in tg_update_disptime() and prepare to cleanup
> flag 'THROTL_TG_PENDING', there are no functional changes.

Does this actually help anything? Given that the heavy part of the operation
remains the same, this might not be much of an optimization. Is there even a
microbench that can show the difference?

Thanks.
Yu Kuai Aug. 17, 2022, 1:38 a.m. UTC | #2 
Hi, Tejun！

在 2022/08/17 4:09, Tejun Heo 写道:
> On Tue, Aug 02, 2022 at 10:04:14PM +0800, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> tg_update_disptime() only need to adjust postion for 'tg' in
>> 'parent_sq', there is no need to call throtl_enqueue/dequeue_tg().
>>
>> Save a little overhead in tg_update_disptime() and prepare to cleanup
>> flag 'THROTL_TG_PENDING', there are no functional changes.
> 
> Does this actually help anything? Given that the heavy part of the operation
> remains the same, this might not be much of an optimization. Is there even a
> microbench that can show the difference?

It's right heavy part remains the same, the patch just remove some
unnecessary operations. And I didn't run benchmark to test that yet.

Thanks,
Kuai
> 
> Thanks.
>
diff mbox series

Patch

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index f7048f87b19f..6b2096e95221 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -520,7 +520,6 @@  static void throtl_rb_erase(struct rb_node *n,
 {
 	rb_erase_cached(n, &parent_sq->pending_tree);
 	RB_CLEAR_NODE(n);
-	--parent_sq->nr_pending;
 }
 
 static void update_min_dispatch_time(struct throtl_service_queue *parent_sq)
@@ -572,7 +571,11 @@  static void throtl_enqueue_tg(struct throtl_grp *tg)
 static void throtl_dequeue_tg(struct throtl_grp *tg)
 {
 	if (tg->flags & THROTL_TG_PENDING) {
-		throtl_rb_erase(&tg->rb_node, tg->service_queue.parent_sq);
+		struct throtl_service_queue *parent_sq =
+			tg->service_queue.parent_sq;
+
+		throtl_rb_erase(&tg->rb_node, parent_sq);
+		--parent_sq->nr_pending;
 		tg->flags &= ~THROTL_TG_PENDING;
 	}
 }
@@ -1045,9 +1048,9 @@  static void tg_update_disptime(struct throtl_grp *tg)
 	disptime = jiffies + min_wait;
 
 	/* Update dispatch time */
-	throtl_dequeue_tg(tg);
+	throtl_rb_erase(&tg->rb_node, tg->service_queue.parent_sq);
 	tg->disptime = disptime;
-	throtl_enqueue_tg(tg);
+	tg_service_queue_add(tg);
 
 	/* see throtl_add_bio_tg() */
 	tg->flags &= ~THROTL_TG_WAS_EMPTY;