cgroups.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hong Zhiguo <honkiko@gmail.com>
To: tj@kernel.org, vgoyal@redhat.com
Cc: cgroups@vger.kernel.org, axboe@kernel.dk,
	linux-kernel@vger.kernel.org,
	Hong Zhiguo <zhiguohong@tencent.com>
Subject: [PATCH v4 2/2] blk-throttle: trim tokens generated for an idle tree
Date: Sun, 20 Oct 2013 20:11:12 +0800	[thread overview]
Message-ID: <1382271072-15664-3-git-send-email-zhiguohong@tencent.com> (raw)
In-Reply-To: <1382271072-15664-1-git-send-email-zhiguohong@tencent.com>

From: Hong Zhiguo <zhiguohong@tencent.com>

Why
====
Pointed out by Vivek: Tokens generated during idle period should
be trimmed. Otherwise a huge bio may be permited immediately.
Overlimit behaviour may be observed during short I/O throughput
test.

Vivek also pointed out: We should not over-trim for hierarchical
groups. Suppose a subtree of groups are idle when a big bio comes.
The token of the child group is trimmed and not enough. So the bio is
queued on the child group. After some jiffies the child group reserved
enough tokens and the bio climbs up. If we trim the parent group at
this time again, this bio will wait too much time than expected.

Analysis
========
When the bio is queued on child group, all it's ancestor groups
becomes non-idle. They should start to reserve tokens for that
bio from this moment. And their reserved tokens before should be
trimmed at this moment.

How
====
service_queue now has a new member nr_queued_tree[2], to represent
the the number of bios waiting on the subtree rooted by this sq.

When a bio is queued on the hierarchy first time, nr_queued_tree
of all ancestors and the child group itself are increased. When a
bio climbs up, nr_queued_tree of the child group is decreased.

When nr_queued_tree turns from zero to one, the tokens reserved
before are trimmed. And after this switch, this group will never
be trimmed to reserve tokens for the bio waiting on it's descendant
group.

Signed-off-by: Hong Zhiguo <zhiguohong@tencent.com>
---
 block/blk-throttle.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 45d4d91..d10a5e1 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -63,6 +63,10 @@ struct throtl_service_queue {
 	 */
 	struct list_head	queued[2];	/* throtl_qnode [READ/WRITE] */
 	unsigned int		nr_queued[2];	/* number of queued bios */
+	/*
+	 * number of bios queued on this subtree
+	 */
+	unsigned int		nr_queued_tree[2];
 
 	/*
 	 * RB tree of active children throtl_grp's, which are sorted by
@@ -699,6 +703,11 @@ static void tg_update_token(struct throtl_grp *tg, bool rw)
 	do_div(token, HZ);
 	tg->bytes_token[rw] += token;
 
+	/* trim token if the whole subtree is idle */
+	if (!tg->service_queue.nr_queued_tree[rw] &&
+	    tg->bytes_token[rw] > THROTL_BURST_BYTES)
+		token = THROTL_BURST_BYTES;
+
 	tg->t_c[rw] = jiffies;
 }
 
@@ -843,6 +852,28 @@ static void throtl_charge_bio(struct throtl_grp *tg, struct bio *bio)
 }
 
 /**
+ * tg_update_nr_queue_tree - update nr_queued_tree of all ancestors
+ * @sq: the target service_queue
+ *
+ * Traverse from @sq to the top throtl_grp, increase their
+ * nr_queued_tree. If one sq has zero nr_queued_tree and now turns to
+ * one, trim the tokens generated before. From now on it will never
+ * get trimmed until same thing happens next time.
+ */
+static void tg_update_nr_queue_tree(struct throtl_service_queue *sq, bool rw)
+{
+	struct throtl_grp *tg;
+
+	while (sq && (tg = sq_to_tg(sq))) {
+		if (!sq->nr_queued_tree[rw])
+			tg_update_token(tg, rw);
+
+		sq->nr_queued_tree[rw]++;
+		sq = sq->parent_sq;
+	}
+}
+
+/**
  * throtl_add_bio_tg - add a bio to the specified throtl_grp
  * @bio: bio to add
  * @qn: qnode to use
@@ -916,6 +947,9 @@ static void tg_dispatch_one_bio(struct throtl_grp *tg, bool rw)
 	bio = throtl_pop_queued(&sq->queued[rw], &tg_to_put);
 	sq->nr_queued[rw]--;
 
+	/* Here's the only place nr_queued_tree get decreased */
+	sq->nr_queued_tree[rw]--;
+
 	throtl_charge_bio(tg, bio);
 
 	/*
@@ -1375,6 +1409,10 @@ bool blk_throtl_bio(struct request_queue *q, struct bio *bio)
 
 	bio_associate_current(bio);
 	tg->td->nr_queued[rw]++;
+
+	/* Here's the only place nr_queued_tree get increased */
+	tg_update_nr_queue_tree(sq, rw);
+
 	throtl_add_bio_tg(bio, qn, tg);
 	throttled = true;
 
-- 
1.8.1.2

  parent reply	other threads:[~2013-10-20 12:11 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-12 10:46 [PATCH] blk-throttle: simplify logic by token bucket algorithm Hong Zhiguo
     [not found] ` <1381574794-7639-1-git-send-email-zhiguohong-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
2013-10-13 12:59   ` Hong zhi guo
2013-10-14  9:09 ` [PATCH v2] " Hong Zhiguo
     [not found]   ` <1381741757-20888-1-git-send-email-zhiguohong-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
2013-10-14 13:36     ` Tejun Heo
2013-10-14 13:47       ` Hong zhi guo
     [not found]       ` <20131014133620.GF4722-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2013-10-14 13:53         ` Hong zhi guo
     [not found]           ` <CAA7+ByWPM2Pizm+dP1AxPM7Ut-w=AtRfD2GkCK-0OVh+C2Twkg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-10-14 13:59             ` Tejun Heo
     [not found]               ` <20131014135929.GH4722-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2013-10-15 12:35                 ` Hong zhi guo
2013-10-15 16:19                   ` Jens Axboe
2013-10-15 13:03         ` Vivek Goyal
2013-10-15 17:32     ` Vivek Goyal
2013-10-16  6:09       ` Hong zhi guo
2013-10-16 14:14         ` Vivek Goyal
2013-10-16 15:47           ` Hong zhi guo
2013-10-16 15:53           ` Tejun Heo
     [not found]             ` <20131016155344.GA10012-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2013-10-16 16:22               ` Vivek Goyal
2013-10-16 16:22               ` Hong zhi guo
2013-10-17 12:17 ` [PATCH v3] " Hong Zhiguo
     [not found]   ` <1382012272-26170-1-git-send-email-zhiguohong-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
2013-10-18 15:55     ` Vivek Goyal
     [not found]       ` <20131018155532.GD2277-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-10-20 12:08         ` Hong zhi guo
2013-10-20 12:11       ` [PATCH v4 0/2] " Hong Zhiguo
2013-10-20 12:11         ` [PATCH v4 1/2] " Hong Zhiguo
2014-04-10 10:07           ` Hong zhi guo
     [not found]             ` <CAA7+ByVJWjfs5HiMsnuum75egghrNQHt2KNNPTWeVa0-FWccaQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-04-10 13:32               ` Vivek Goyal
2013-10-20 12:11         ` Hong Zhiguo [this message]
     [not found]           ` <1382271072-15664-3-git-send-email-zhiguohong-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
2013-10-22 21:02             ` [PATCH v4 2/2] blk-throttle: trim tokens generated for an idle tree Vivek Goyal
     [not found]               ` <20131022210232.GB2884-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-10-23  3:30                 ` Hong zhi guo
2013-10-28  5:08               ` Hong zhi guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1382271072-15664-3-git-send-email-zhiguohong@tencent.com \
    --to=honkiko@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=cgroups@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=vgoyal@redhat.com \
    --cc=zhiguohong@tencent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).