Re: [PATCH][v2] Changes to more io-controller stats patches to address review comments.

Previous thread: [PATCH 1/2] RFC: iscsi ibft: separate ibft parsing from sysfs interface by michaelc on Monday, April 12, 2010 - 11:06 am. (4 messages)

Next thread: [PATCH 2/2] ipc semaphores: order wakeups based on waiter CPU by Chris Mason on Monday, April 12, 2010 - 11:49 am. (2 messages)
From: Divyesh Shah
Date: Monday, April 12, 2010 - 11:41 am

Changelog from v1:
o Call blkiocg_update_idle_time_stats() at cfq_rq_enqueued() instead of at
  dispatch time.

Changelog from original patchset: (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 |   20 +++++++++-----------
 3 files changed, 28 insertions(+), 32 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 ...
From: Vivek Goyal
Date: Monday, April 12, 2010 - 3:11 pm

Looks good to me.

Acked-by: Vivek Goyal <vgoyal@redhat.com>

Thanks
--

From: Divyesh Shah
Date: Tuesday, April 13, 2010 - 10:26 am

Hi Jens,
       If this looks ok to you, can you please queue it up? Btw, after
Gui's patches there is 1 merge conflict in your for-2.6.35 branch when
applying this patch but it seems pretty straightforward to resolve. In
case you'd prefer a clean patch that takes care of the merge conflict
please let me know.

Thanks,
Divyesh

--

From: Jens Axboe
Date: Tuesday, April 13, 2010 - 11:15 am

I hand applied that one hunk, and committed it.

But...

block/blk-cgroup.c:271: error: redefinition of ?blkiocg_set_start_empty_time?
block/blk-cgroup.h:208: note: previous definition of ?blkiocg_set_start_empty_time? was here
block/blk-cgroup.c: In function ?blkiocg_set_start_empty_time?:
block/blk-cgroup.c:291: error: implicit declaration of function ?blkio_blkg_empty?
block/blk-cgroup.c:292: error: ?struct blkio_group_stats? has no member named ?start_empty_time?
block/blk-cgroup.c:293: error: implicit declaration of function ?blkio_mark_blkg_empty?
make[1]: *** [block/blk-cgroup.o] Error 1
make: *** [block] Error 2

I merged mainline into for-2.6.35 to resolve a merge conflict, but that
was trivial.

-- 
Jens Axboe

--

From: Divyesh Shah
Date: Tuesday, April 13, 2010 - 2:28 pm

Ahh.. I see this now with CONFIG_DEBUG_CFQ_IOSCHED not set.
blkiocg_set_empty_start_time() should be within the #ifdef

block/cfq-iosched.c: In function 'cfq_forced_dispatch':
block/cfq-iosched.c:2234: error: too few arguments to function
'cfq_slice_expired'

Another compile error after fixing this was due to the merge from
mainline. cfq_slice_expired() now has an extra argument. Fixed that
too. Will send a patch with the 2 fixes in a bit. Sorry for the
--

Previous thread: [PATCH 1/2] RFC: iscsi ibft: separate ibft parsing from sysfs interface by michaelc on Monday, April 12, 2010 - 11:06 am. (4 messages)

Next thread: [PATCH 2/2] ipc semaphores: order wakeups based on waiter CPU by Chris Mason on Monday, April 12, 2010 - 11:49 am. (2 messages)