[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <w2naf41c7c41004131026s3b81d78cl6003df56c32da992@mail.gmail.com>
Date: Tue, 13 Apr 2010 10:26:27 -0700
From: Divyesh Shah <dpshah@...gle.com>
To: Vivek Goyal <vgoyal@...hat.com>
Cc: jens.axboe@...cle.com, linux-kernel@...r.kernel.org,
nauman@...gle.com, ctalbott@...gle.com
Subject: Re: [PATCH][v2] Changes to more io-controller stats patches to
address review comments.
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
On Mon, Apr 12, 2010 at 3:11 PM, Vivek Goyal <vgoyal@...hat.com> wrote:
> On Mon, Apr 12, 2010 at 11:41:49AM -0700, Divyesh Shah wrote:
>> 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@...gle.com>
>> ---
>
> Looks good to me.
>
> Acked-by: Vivek Goyal <vgoyal@...hat.com>
>
> Thanks
> Vivek
>
>>
>> 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 blkiocg_update_request_add_stats(struct blkio_group *blkg,
>> +void blkiocg_update_io_add_stats(struct blkio_group *blkg,
>> struct blkio_group *curr_blkg, bool direction,
>> bool sync)
>> {
>> @@ -209,9 +216,9 @@ void blkiocg_update_request_add_stats(struct blkio_group *blkg,
>> blkio_set_start_group_wait_time(blkg, curr_blkg);
>> spin_unlock_irqrestore(&blkg->stats_lock, flags);
>> }
>> -EXPORT_SYMBOL_GPL(blkiocg_update_request_add_stats);
>> +EXPORT_SYMBOL_GPL(blkiocg_update_io_add_stats);
>>
>> -void blkiocg_update_request_remove_stats(struct blkio_group *blkg,
>> +void blkiocg_update_io_remove_stats(struct blkio_group *blkg,
>> bool direction, bool sync)
>> {
>> unsigned long flags;
>> @@ -221,7 +228,7 @@ void blkiocg_update_request_remove_stats(struct blkio_group *blkg,
>> direction, sync);
>> spin_unlock_irqrestore(&blkg->stats_lock, flags);
>> }
>> -EXPORT_SYMBOL_GPL(blkiocg_update_request_remove_stats);
>> +EXPORT_SYMBOL_GPL(blkiocg_update_io_remove_stats);
>>
>> void blkiocg_update_timeslice_used(struct blkio_group *blkg, unsigned long time)
>> {
>> @@ -602,15 +609,6 @@ SHOW_FUNCTION_PER_GROUP(empty_time, BLKIO_STAT_EMPTY_TIME, 0);
>> #endif
>> #undef SHOW_FUNCTION_PER_GROUP
>>
>> -#ifdef CONFIG_DEBUG_BLK_CGROUP
>> -void blkiocg_update_dequeue_stats(struct blkio_group *blkg,
>> - unsigned long dequeue)
>> -{
>> - blkg->stats.dequeue += dequeue;
>> -}
>> -EXPORT_SYMBOL_GPL(blkiocg_update_dequeue_stats);
>> -#endif
>> -
>> struct cftype blkio_files[] = {
>> {
>> .name = "weight",
>> diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
>> index bfce085..18e031a 100644
>> --- a/block/blk-cgroup.h
>> +++ b/block/blk-cgroup.h
>> @@ -159,7 +159,7 @@ static inline char *blkg_path(struct blkio_group *blkg)
>> {
>> return blkg->path;
>> }
>> -void blkiocg_update_set_active_queue_stats(struct blkio_group *blkg);
>> +void blkiocg_update_avg_queue_size_stats(struct blkio_group *blkg);
>> void blkiocg_update_dequeue_stats(struct blkio_group *blkg,
>> unsigned long dequeue);
>> void blkiocg_update_set_idle_time_stats(struct blkio_group *blkg);
>> @@ -188,7 +188,7 @@ BLKG_FLAG_FNS(empty)
>> #undef BLKG_FLAG_FNS
>> #else
>> static inline char *blkg_path(struct blkio_group *blkg) { return NULL; }
>> -static inline void blkiocg_update_set_active_queue_stats(
>> +static inline void blkiocg_update_avg_queue_size_stats(
>> struct blkio_group *blkg) {}
>> static inline void blkiocg_update_dequeue_stats(struct blkio_group *blkg,
>> unsigned long dequeue) {}
>> @@ -216,9 +216,9 @@ void blkiocg_update_completion_stats(struct blkio_group *blkg,
>> uint64_t start_time, uint64_t io_start_time, bool direction, bool sync);
>> void blkiocg_update_io_merged_stats(struct blkio_group *blkg, bool direction,
>> bool sync);
>> -void blkiocg_update_request_add_stats(struct blkio_group *blkg,
>> +void blkiocg_update_io_add_stats(struct blkio_group *blkg,
>> struct blkio_group *curr_blkg, bool direction, bool sync);
>> -void blkiocg_update_request_remove_stats(struct blkio_group *blkg,
>> +void blkiocg_update_io_remove_stats(struct blkio_group *blkg,
>> bool direction, bool sync);
>> #else
>> struct cgroup;
>> @@ -243,9 +243,9 @@ static inline void blkiocg_update_completion_stats(struct blkio_group *blkg,
>> bool sync) {}
>> static inline void blkiocg_update_io_merged_stats(struct blkio_group *blkg,
>> bool direction, bool sync) {}
>> -static inline void blkiocg_update_request_add_stats(struct blkio_group *blkg,
>> +static inline void blkiocg_update_io_add_stats(struct blkio_group *blkg,
>> struct blkio_group *curr_blkg, bool direction, bool sync) {}
>> -static inline void blkiocg_update_request_remove_stats(struct blkio_group *blkg,
>> +static inline void blkiocg_update_io_remove_stats(struct blkio_group *blkg,
>> bool direction, bool sync) {}
>> #endif
>> #endif /* _BLK_CGROUP_H */
>> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
>> index b6e095c..cda6b29 100644
>> --- a/block/cfq-iosched.c
>> +++ b/block/cfq-iosched.c
>> @@ -1381,10 +1381,10 @@ static void cfq_reposition_rq_rb(struct cfq_queue *cfqq, struct request *rq)
>> {
>> elv_rb_del(&cfqq->sort_list, rq);
>> cfqq->queued[rq_is_sync(rq)]--;
>> - blkiocg_update_request_remove_stats(&cfqq->cfqg->blkg, rq_data_dir(rq),
>> + blkiocg_update_io_remove_stats(&cfqq->cfqg->blkg, rq_data_dir(rq),
>> rq_is_sync(rq));
>> cfq_add_rq_rb(rq);
>> - blkiocg_update_request_add_stats(
>> + blkiocg_update_io_add_stats(
>> &cfqq->cfqg->blkg, &cfqq->cfqd->serving_group->blkg,
>> rq_data_dir(rq), rq_is_sync(rq));
>> }
>> @@ -1442,7 +1442,7 @@ static void cfq_remove_request(struct request *rq)
>> cfq_del_rq_rb(rq);
>>
>> cfqq->cfqd->rq_queued--;
>> - blkiocg_update_request_remove_stats(&cfqq->cfqg->blkg, rq_data_dir(rq),
>> + blkiocg_update_io_remove_stats(&cfqq->cfqg->blkg, rq_data_dir(rq),
>> rq_is_sync(rq));
>> if (rq_is_meta(rq)) {
>> WARN_ON(!cfqq->meta_pending);
>> @@ -1541,7 +1541,7 @@ static void __cfq_set_active_queue(struct cfq_data *cfqd,
>> if (cfqq) {
>> cfq_log_cfqq(cfqd, cfqq, "set_active wl_prio:%d wl_type:%d",
>> cfqd->serving_prio, cfqd->serving_type);
>> - blkiocg_update_set_active_queue_stats(&cfqq->cfqg->blkg);
>> + blkiocg_update_avg_queue_size_stats(&cfqq->cfqg->blkg);
>> cfqq->slice_start = 0;
>> cfqq->dispatch_start = jiffies;
>> cfqq->allocated_slice = 0;
>> @@ -2395,11 +2395,6 @@ static int cfq_dispatch_requests(struct request_queue *q, int force)
>> }
>>
>> cfq_log_cfqq(cfqd, cfqq, "dispatched a request");
>> - /*
>> - * This is needed since we don't exactly match the mod_timer() and
>> - * del_timer() calls in CFQ.
>> - */
>> - blkiocg_update_idle_time_stats(&cfqq->cfqg->blkg);
>> return 1;
>> }
>>
>> @@ -3208,8 +3203,11 @@ cfq_rq_enqueued(struct cfq_data *cfqd, struct cfq_queue *cfqq,
>> cfq_del_timer(cfqd, cfqq);
>> cfq_clear_cfqq_wait_request(cfqq);
>> __blk_run_queue(cfqd->queue);
>> - } else
>> + } else {
>> + blkiocg_update_idle_time_stats(
>> + &cfqq->cfqg->blkg);
>> cfq_mark_cfqq_must_dispatch(cfqq);
>> + }
>> }
>> } else if (cfq_should_preempt(cfqd, cfqq, rq)) {
>> /*
>> @@ -3235,7 +3233,7 @@ static void cfq_insert_request(struct request_queue *q, struct request *rq)
>> list_add_tail(&rq->queuelist, &cfqq->fifo);
>> cfq_add_rq_rb(rq);
>>
>> - blkiocg_update_request_add_stats(&cfqq->cfqg->blkg,
>> + blkiocg_update_io_add_stats(&cfqq->cfqg->blkg,
>> &cfqd->serving_group->blkg, rq_data_dir(rq),
>> rq_is_sync(rq));
>> cfq_rq_enqueued(cfqd, cfqq, rq);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists