Re: [PATCH] blkio: Changes to more io-controller stats patches to address

Previous thread: [PATCH] tcp: add setsockopt to disable slow start after idle by Cristian KLEIN on Friday, April 9, 2010 - 6:30 pm. (9 messages)

Next thread: [PATCH] microblaze: Quiet section mismatch warnings by Steven J. Magnani on Friday, April 9, 2010 - 8:03 pm. (2 messages)
From: Divyesh Shah
Date: Friday, April 9, 2010 - 7:35 pm

review comments.

Changelog: (in response to Vivek Goyal's comments)
o group blkiocg_update_blkio_group_dequeue_stats() with other DEBUG functions
o rename blkiocg_update_set_active_queue_stats() to
  blkiocg_update_avg_queue_size_stats()
o s/request/io/ in blkiocg_update_request_add_stats() and
  blkiocg_update_request_remove_stats()
o Call cfq_del_timer() at request dispatch() instead of
  blkiocg_update_idle_time_stats()

Signed-off-by: Divyesh Shah<dpshah@google.com>
---

 block/blk-cgroup.c  |   28 +++++++++++++---------------
 block/blk-cgroup.h  |   12 ++++++------
 block/cfq-iosched.c |   16 ++++++----------
 3 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 1ecff7a..fd428c1 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -175,7 +175,7 @@ void blkiocg_update_idle_time_stats(struct blkio_group *blkg)
 }
 EXPORT_SYMBOL_GPL(blkiocg_update_idle_time_stats);
 
-void blkiocg_update_set_active_queue_stats(struct blkio_group *blkg)
+void blkiocg_update_avg_queue_size_stats(struct blkio_group *blkg)
 {
 	unsigned long flags;
 	struct blkio_group_stats *stats;
@@ -189,14 +189,21 @@ void blkiocg_update_set_active_queue_stats(struct blkio_group *blkg)
 	blkio_update_group_wait_time(stats);
 	spin_unlock_irqrestore(&blkg->stats_lock, flags);
 }
-EXPORT_SYMBOL_GPL(blkiocg_update_set_active_queue_stats);
+EXPORT_SYMBOL_GPL(blkiocg_update_avg_queue_size_stats);
+
+void blkiocg_update_dequeue_stats(struct blkio_group *blkg,
+			unsigned long dequeue)
+{
+	blkg->stats.dequeue += dequeue;
+}
+EXPORT_SYMBOL_GPL(blkiocg_update_dequeue_stats);
 #else
 static inline void blkio_set_start_group_wait_time(struct blkio_group *blkg,
 					struct blkio_group *curr_blkg) {}
 static inline void blkio_end_empty_time(struct blkio_group_stats *stats) {}
 #endif
 
-void blkiocg_update_request_add_stats(struct blkio_group *blkg,
+void blkiocg_update_io_add_stats(struct blkio_group *blkg,
 			struct blkio_group ...
From: Vivek Goyal
Date: Monday, April 12, 2010 - 7:30 am

I am trying to think that why do we need this call here. What is that path
which does not delete the timer upon receiving request and lets fix that.

One of the possible path when you got the request but still leave the
timer on is following.

cfq_rq_enqueued()
	if(request_not_big_enough)
		leave_timer_on
		cfq_mark_cfqq_must_dispatch(cfqq);

I guess we need to leave timer on so that if an unplug does not come in, a
timer expirty will dispatch the request from the queue.

In this case we can probably call here blkiocg_update_idle_time_stats()
directly. Anyway, that's the right thing to do otherwise our idle time
stats are little wrong as we got a request from the application and idle
timer is over. Now this is additional wait time enforced by cfq so that
lots of small requests are not dispatched to disk.

Thanks
Vivek
--

From: Divyesh Shah
Date: Monday, April 12, 2010 - 11:37 am

Previous thread: [PATCH] tcp: add setsockopt to disable slow start after idle by Cristian KLEIN on Friday, April 9, 2010 - 6:30 pm. (9 messages)

Next thread: [PATCH] microblaze: Quiet section mismatch warnings by Steven J. Magnani on Friday, April 9, 2010 - 8:03 pm. (2 messages)