public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Jens Axboe <jens.axboe@oracle.com>
Cc: Corrado Zoccolo <czoccolo@gmail.com>,
	linux-kernel@vger.kernel.org, nauman@google.com,
	dpshah@google.com, lizf@cn.fujitsu.com, ryov@valinux.co.jp,
	fernando@oss.ntt.co.jp, s-uchida@ap.jp.nec.com,
	taka@valinux.co.jp, guijianfeng@cn.fujitsu.com,
	jmoyer@redhat.com, balbir@linux.vnet.ibm.com,
	righi.andrea@gmail.com, m-ikeda@ds.jp.nec.com,
	akpm@linux-foundation.org, riel@redhat.com,
	kamezawa.hiroyu@jp.fujitsu.com
Subject: Re: [PATCH 03/16] blkio: Keep queue on service tree until we expire   it
Date: Fri, 13 Nov 2009 10:05:12 -0500	[thread overview]
Message-ID: <20091113150512.GB17076@redhat.com> (raw)
In-Reply-To: <20091113104808.GW8742@kernel.dk>

On Fri, Nov 13, 2009 at 11:48:08AM +0100, Jens Axboe wrote:
> On Fri, Nov 13 2009, Corrado Zoccolo wrote:
> > Hi Vivek,
> > the following is probably not on a critical path, but maybe it can be
> > written more efficiently.
> > Now, it will cicle through all service trees multiple times, to
> > retrieve the queues for the last one.
> > What about having a cfq_for_each_queue that takes a function pointer
> > and will apply it to all of them?
> 
> I thought the same when reading this. Or perhaps have a small bitmap
> instead/with the counter to avoid looping around known empty groups.

Hi Jens,

Empty groups will not be on the group service tree. Only exception is
the group belonging to current active queue where queue might be empty
but still on service tree because we might be idling on it.

So we don't have to worry about looping through empty groups.

> 
> I want to start merging the initial prep patches soon, so we can both
> cut back on the size of your patchset while getting the simpler bits in.
> Unfortunately I had to stop here, but with a few corrections it can be
> merged too.

Now we are left with the issue of looping through various empty service
trees with-in group. We already keep a count of busy queues in each
service tree. One simple way is to just check for that count and if tree
is empty, simply return back. I have modified cfq_rb_first() to check for
count first before it tries to do rb_next().

Please let me know if you think that this is not good enough and we need
to maintain a bit map of busy service trees in a group.


o If service tree is emtpy, return NULL.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 block/cfq-iosched.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux7/block/cfq-iosched.c
===================================================================
--- linux7.orig/block/cfq-iosched.c	2009-11-12 18:02:03.000000000 -0500
+++ linux7/block/cfq-iosched.c	2009-11-13 09:48:38.000000000 -0500
@@ -664,6 +664,10 @@ cfq_choose_req(struct cfq_data *cfqd, st
  */
 static struct cfq_queue *cfq_rb_first(struct cfq_rb_root *root)
 {
+	/* Service tree is empty */
+	if (!root->count)
+		return NULL;
+
 	if (!root->left)
 		root->left = rb_first(&root->rb);
 

  reply	other threads:[~2009-11-13 15:05 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-12 23:32 [RFC] Block IO Controller V2 Vivek Goyal
2009-11-12 23:32 ` [PATCH 01/16] blkio: Documentation Vivek Goyal
2009-11-13 10:48   ` Jens Axboe
2009-11-13 15:18     ` Vivek Goyal
2009-11-12 23:32 ` [PATCH 02/16] blkio: Introduce the notion of cfq groups Vivek Goyal
2009-11-12 23:32 ` [PATCH 03/16] blkio: Keep queue on service tree until we expire it Vivek Goyal
2009-11-13 10:39   ` Corrado Zoccolo
2009-11-13 10:48     ` Jens Axboe
2009-11-13 15:05       ` Vivek Goyal [this message]
2009-11-13 18:44         ` Jens Axboe
2009-11-12 23:32 ` [PATCH 04/16] blkio: Introduce the root service tree for cfq groups Vivek Goyal
2009-11-12 23:32 ` [PATCH 05/16] blkio: Implement per cfq group latency target and busy queue avg Vivek Goyal
2009-11-13 10:46   ` Corrado Zoccolo
2009-11-13 15:18     ` Vivek Goyal
2009-11-13 16:15       ` Vivek Goyal
2009-11-13 18:40         ` Corrado Zoccolo
2009-11-13 19:26           ` Vivek Goyal
2009-11-13 19:38             ` Corrado Zoccolo
2009-11-12 23:32 ` [PATCH 06/16] blkio: Introduce blkio controller cgroup interface Vivek Goyal
2009-11-12 23:32 ` [PATCH 07/16] blkio: Introduce per cfq group weights and vdisktime calculations Vivek Goyal
2009-11-12 23:32 ` [PATCH 08/16] blkio: Group time used accounting and workload context save restore Vivek Goyal
2009-11-12 23:32 ` [PATCH 09/16] blkio: Dynamic cfq group creation based on cgroup tasks belongs to Vivek Goyal
2009-11-12 23:32 ` [PATCH 10/16] blkio: Take care of cgroup deletion and cfq group reference counting Vivek Goyal
2009-11-12 23:32 ` [PATCH 11/16] blkio: Some debugging aids for CFQ Vivek Goyal
2009-11-12 23:32 ` [PATCH 12/16] blkio: Export disk time and sectors used by a group to user space Vivek Goyal
2009-11-12 23:32 ` [PATCH 13/16] blkio: Provide some isolation between groups Vivek Goyal
2009-11-12 23:32 ` [PATCH 14/16] blkio: Idle on a group for some time on rotational media Vivek Goyal
2009-11-13 10:58   ` Corrado Zoccolo
2009-11-13 15:37     ` Vivek Goyal
2009-11-12 23:32 ` [PATCH 15/16] blkio: Drop the reference to queue once the task changes cgroup Vivek Goyal
2009-11-12 23:32 ` [PATCH 16/16] blkio: Propagate cgroup weight updation to cfq groups Vivek Goyal

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=20091113150512.GB17076@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=czoccolo@gmail.com \
    --cc=dpshah@google.com \
    --cc=fernando@oss.ntt.co.jp \
    --cc=guijianfeng@cn.fujitsu.com \
    --cc=jens.axboe@oracle.com \
    --cc=jmoyer@redhat.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=m-ikeda@ds.jp.nec.com \
    --cc=nauman@google.com \
    --cc=riel@redhat.com \
    --cc=righi.andrea@gmail.com \
    --cc=ryov@valinux.co.jp \
    --cc=s-uchida@ap.jp.nec.com \
    --cc=taka@valinux.co.jp \
    /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