* [PATCH] cfq-iosched: requests "in flight" vs "in driver" clarification
2009-12-30 21:16 [PATCH] cfq-iosched: replace sync_flight by rq_in_driver[BLK_RW_SYNC] Jens Axboe
@ 2009-12-30 22:24 ` Corrado Zoccolo
0 siblings, 0 replies; 4+ messages in thread
From: Corrado Zoccolo @ 2009-12-30 22:24 UTC (permalink / raw)
To: Jens Axboe
Cc: Linux-Kernel, Jeff Moyer, Vivek Goyal, Shaohua Li, Gui Jianfeng,
Corrado Zoccolo
Counters for requests "in flight" and "in driver" are used asymmetrically
in cfq_may_dispatch, and have slightly different meaning.
We split the rq_in_flight counter (was sync_flight) to count both sync
and async requests, in order to use this one, which is more accurate in
some corner cases.
The rq_in_driver counter is coalesced, since individual sync/async counts
are not used any more.
Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
---
block/cfq-iosched.c | 44 ++++++++++++++++++--------------------------
1 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 7da9391..c6d5678 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -227,8 +227,8 @@ struct cfq_data {
unsigned int busy_queues;
- int rq_in_driver[2];
- int sync_flight;
+ int rq_in_driver;
+ int rq_in_flight[2];
/*
* queue-depth detection
@@ -419,11 +419,6 @@ static struct cfq_queue *cfq_get_queue(struct cfq_data *, bool,
static struct cfq_io_context *cfq_cic_lookup(struct cfq_data *,
struct io_context *);
-static inline int rq_in_driver(struct cfq_data *cfqd)
-{
- return cfqd->rq_in_driver[0] + cfqd->rq_in_driver[1];
-}
-
static inline struct cfq_queue *cic_to_cfqq(struct cfq_io_context *cic,
bool is_sync)
{
@@ -1423,9 +1418,9 @@ static void cfq_activate_request(struct request_queue *q, struct request *rq)
{
struct cfq_data *cfqd = q->elevator->elevator_data;
- cfqd->rq_in_driver[rq_is_sync(rq)]++;
+ cfqd->rq_in_driver++;
cfq_log_cfqq(cfqd, RQ_CFQQ(rq), "activate rq, drv=%d",
- rq_in_driver(cfqd));
+ cfqd->rq_in_driver);
cfqd->last_position = blk_rq_pos(rq) + blk_rq_sectors(rq);
}
@@ -1433,12 +1428,11 @@ static void cfq_activate_request(struct request_queue *q, struct request *rq)
static void cfq_deactivate_request(struct request_queue *q, struct request *rq)
{
struct cfq_data *cfqd = q->elevator->elevator_data;
- const int sync = rq_is_sync(rq);
- WARN_ON(!cfqd->rq_in_driver[sync]);
- cfqd->rq_in_driver[sync]--;
+ WARN_ON(!cfqd->rq_in_driver);
+ cfqd->rq_in_driver--;
cfq_log_cfqq(cfqd, RQ_CFQQ(rq), "deactivate rq, drv=%d",
- rq_in_driver(cfqd));
+ cfqd->rq_in_driver);
}
static void cfq_remove_request(struct request *rq)
@@ -1876,8 +1870,7 @@ static void cfq_dispatch_insert(struct request_queue *q, struct request *rq)
cfqq->dispatched++;
elv_dispatch_sort(q, rq);
- if (cfq_cfqq_sync(cfqq))
- cfqd->sync_flight++;
+ cfqd->rq_in_flight[cfq_cfqq_sync(cfqq)]++;
cfqq->nr_sectors += blk_rq_sectors(rq);
}
@@ -2224,13 +2217,13 @@ static bool cfq_may_dispatch(struct cfq_data *cfqd, struct cfq_queue *cfqq)
/*
* Drain async requests before we start sync IO
*/
- if (cfq_should_idle(cfqd, cfqq) && cfqd->rq_in_driver[BLK_RW_ASYNC])
+ if (cfq_should_idle(cfqd, cfqq) && cfqd->rq_in_flight[BLK_RW_ASYNC])
return false;
/*
* If this is an async queue and we have sync IO in flight, let it wait
*/
- if (cfqd->sync_flight && !cfq_cfqq_sync(cfqq))
+ if (cfqd->rq_in_flight[BLK_RW_SYNC] && !cfq_cfqq_sync(cfqq))
return false;
max_dispatch = cfqd->cfq_quantum;
@@ -3220,14 +3213,14 @@ static void cfq_update_hw_tag(struct cfq_data *cfqd)
{
struct cfq_queue *cfqq = cfqd->active_queue;
- if (rq_in_driver(cfqd) > cfqd->hw_tag_est_depth)
- cfqd->hw_tag_est_depth = rq_in_driver(cfqd);
+ if (cfqd->rq_in_driver > cfqd->hw_tag_est_depth)
+ cfqd->hw_tag_est_depth = cfqd->rq_in_driver;
if (cfqd->hw_tag == 1)
return;
if (cfqd->rq_queued <= CFQ_HW_QUEUE_MIN &&
- rq_in_driver(cfqd) <= CFQ_HW_QUEUE_MIN)
+ cfqd->rq_in_driver <= CFQ_HW_QUEUE_MIN)
return;
/*
@@ -3237,7 +3230,7 @@ static void cfq_update_hw_tag(struct cfq_data *cfqd)
*/
if (cfqq && cfq_cfqq_idle_window(cfqq) &&
cfqq->dispatched + cfqq->queued[0] + cfqq->queued[1] <
- CFQ_HW_QUEUE_MIN && rq_in_driver(cfqd) < CFQ_HW_QUEUE_MIN)
+ CFQ_HW_QUEUE_MIN && cfqd->rq_in_driver < CFQ_HW_QUEUE_MIN)
return;
if (cfqd->hw_tag_samples++ < 50)
@@ -3290,13 +3283,12 @@ static void cfq_completed_request(struct request_queue *q, struct request *rq)
cfq_update_hw_tag(cfqd);
- WARN_ON(!cfqd->rq_in_driver[sync]);
+ WARN_ON(!cfqd->rq_in_driver);
WARN_ON(!cfqq->dispatched);
- cfqd->rq_in_driver[sync]--;
+ cfqd->rq_in_driver--;
cfqq->dispatched--;
- if (cfq_cfqq_sync(cfqq))
- cfqd->sync_flight--;
+ cfqd->rq_in_flight[cfq_cfqq_sync(cfqq)]--;
if (sync) {
RQ_CIC(rq)->last_end_request = now;
@@ -3350,7 +3342,7 @@ static void cfq_completed_request(struct request_queue *q, struct request *rq)
}
}
- if (!rq_in_driver(cfqd))
+ if (!cfqd->rq_in_driver)
cfq_schedule_dispatch(cfqd);
}
--
1.6.4.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] cfq-iosched: requests "in flight" vs "in driver" clarification
@ 2010-02-27 16:03 Corrado Zoccolo
2010-02-28 18:44 ` Jens Axboe
2010-03-01 14:11 ` Vivek Goyal
0 siblings, 2 replies; 4+ messages in thread
From: Corrado Zoccolo @ 2010-02-27 16:03 UTC (permalink / raw)
To: Jens Axboe
Cc: Linux-Kernel, Jeff Moyer, Vivek Goyal, Shaohua Li, Gui Jianfeng,
Corrado Zoccolo
Counters for requests "in flight" and "in driver" are used asymmetrically
in cfq_may_dispatch, and have slightly different meaning.
We split the rq_in_flight counter (was sync_flight) to count both sync
and async requests, in order to use this one, which is more accurate in
some corner cases.
The rq_in_driver counter is coalesced, since individual sync/async counts
are not used any more.
Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
---
block/cfq-iosched.c | 44 ++++++++++++++++++--------------------------
1 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 10eb286..eed649c 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -223,8 +223,8 @@ struct cfq_data {
unsigned int busy_queues;
- int rq_in_driver[2];
- int sync_flight;
+ int rq_in_driver;
+ int rq_in_flight[2];
/*
* queue-depth detection
@@ -417,11 +417,6 @@ static struct cfq_queue *cfq_get_queue(struct cfq_data *, bool,
static struct cfq_io_context *cfq_cic_lookup(struct cfq_data *,
struct io_context *);
-static inline int rq_in_driver(struct cfq_data *cfqd)
-{
- return cfqd->rq_in_driver[0] + cfqd->rq_in_driver[1];
-}
-
static inline struct cfq_queue *cic_to_cfqq(struct cfq_io_context *cic,
bool is_sync)
{
@@ -1415,9 +1410,9 @@ static void cfq_activate_request(struct request_queue *q, struct request *rq)
{
struct cfq_data *cfqd = q->elevator->elevator_data;
- cfqd->rq_in_driver[rq_is_sync(rq)]++;
+ cfqd->rq_in_driver++;
cfq_log_cfqq(cfqd, RQ_CFQQ(rq), "activate rq, drv=%d",
- rq_in_driver(cfqd));
+ cfqd->rq_in_driver);
cfqd->last_position = blk_rq_pos(rq) + blk_rq_sectors(rq);
}
@@ -1425,12 +1420,11 @@ static void cfq_activate_request(struct request_queue *q, struct request *rq)
static void cfq_deactivate_request(struct request_queue *q, struct request *rq)
{
struct cfq_data *cfqd = q->elevator->elevator_data;
- const int sync = rq_is_sync(rq);
- WARN_ON(!cfqd->rq_in_driver[sync]);
- cfqd->rq_in_driver[sync]--;
+ WARN_ON(!cfqd->rq_in_driver);
+ cfqd->rq_in_driver--;
cfq_log_cfqq(cfqd, RQ_CFQQ(rq), "deactivate rq, drv=%d",
- rq_in_driver(cfqd));
+ cfqd->rq_in_driver);
}
static void cfq_remove_request(struct request *rq)
@@ -1873,8 +1867,7 @@ static void cfq_dispatch_insert(struct request_queue *q, struct request *rq)
cfqq->dispatched++;
elv_dispatch_sort(q, rq);
- if (cfq_cfqq_sync(cfqq))
- cfqd->sync_flight++;
+ cfqd->rq_in_flight[cfq_cfqq_sync(cfqq)]++;
cfqq->nr_sectors += blk_rq_sectors(rq);
}
@@ -2221,13 +2214,13 @@ static bool cfq_may_dispatch(struct cfq_data *cfqd, struct cfq_queue *cfqq)
/*
* Drain async requests before we start sync IO
*/
- if (cfq_should_idle(cfqd, cfqq) && cfqd->rq_in_driver[BLK_RW_ASYNC])
+ if (cfq_should_idle(cfqd, cfqq) && cfqd->rq_in_flight[BLK_RW_ASYNC])
return false;
/*
* If this is an async queue and we have sync IO in flight, let it wait
*/
- if (cfqd->sync_flight && !cfq_cfqq_sync(cfqq))
+ if (cfqd->rq_in_flight[BLK_RW_SYNC] && !cfq_cfqq_sync(cfqq))
return false;
max_dispatch = cfqd->cfq_quantum;
@@ -3210,14 +3203,14 @@ static void cfq_update_hw_tag(struct cfq_data *cfqd)
{
struct cfq_queue *cfqq = cfqd->active_queue;
- if (rq_in_driver(cfqd) > cfqd->hw_tag_est_depth)
- cfqd->hw_tag_est_depth = rq_in_driver(cfqd);
+ if (cfqd->rq_in_driver > cfqd->hw_tag_est_depth)
+ cfqd->hw_tag_est_depth = cfqd->rq_in_driver;
if (cfqd->hw_tag == 1)
return;
if (cfqd->rq_queued <= CFQ_HW_QUEUE_MIN &&
- rq_in_driver(cfqd) <= CFQ_HW_QUEUE_MIN)
+ cfqd->rq_in_driver <= CFQ_HW_QUEUE_MIN)
return;
/*
@@ -3227,7 +3220,7 @@ static void cfq_update_hw_tag(struct cfq_data *cfqd)
*/
if (cfqq && cfq_cfqq_idle_window(cfqq) &&
cfqq->dispatched + cfqq->queued[0] + cfqq->queued[1] <
- CFQ_HW_QUEUE_MIN && rq_in_driver(cfqd) < CFQ_HW_QUEUE_MIN)
+ CFQ_HW_QUEUE_MIN && cfqd->rq_in_driver < CFQ_HW_QUEUE_MIN)
return;
if (cfqd->hw_tag_samples++ < 50)
@@ -3280,13 +3273,12 @@ static void cfq_completed_request(struct request_queue *q, struct request *rq)
cfq_update_hw_tag(cfqd);
- WARN_ON(!cfqd->rq_in_driver[sync]);
+ WARN_ON(!cfqd->rq_in_driver);
WARN_ON(!cfqq->dispatched);
- cfqd->rq_in_driver[sync]--;
+ cfqd->rq_in_driver--;
cfqq->dispatched--;
- if (cfq_cfqq_sync(cfqq))
- cfqd->sync_flight--;
+ cfqd->rq_in_flight[cfq_cfqq_sync(cfqq)]--;
if (sync) {
RQ_CIC(rq)->last_end_request = now;
@@ -3340,7 +3332,7 @@ static void cfq_completed_request(struct request_queue *q, struct request *rq)
}
}
- if (!rq_in_driver(cfqd))
+ if (!cfqd->rq_in_driver)
cfq_schedule_dispatch(cfqd);
}
--
1.6.4.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] cfq-iosched: requests "in flight" vs "in driver" clarification
2010-02-27 16:03 [PATCH] cfq-iosched: requests "in flight" vs "in driver" clarification Corrado Zoccolo
@ 2010-02-28 18:44 ` Jens Axboe
2010-03-01 14:11 ` Vivek Goyal
1 sibling, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2010-02-28 18:44 UTC (permalink / raw)
To: Corrado Zoccolo
Cc: Linux-Kernel, Jeff Moyer, Vivek Goyal, Shaohua Li, Gui Jianfeng
On Sat, Feb 27 2010, Corrado Zoccolo wrote:
> Counters for requests "in flight" and "in driver" are used asymmetrically
> in cfq_may_dispatch, and have slightly different meaning.
> We split the rq_in_flight counter (was sync_flight) to count both sync
> and async requests, in order to use this one, which is more accurate in
> some corner cases.
> The rq_in_driver counter is coalesced, since individual sync/async counts
> are not used any more.
Thanks, applied.
--
Jens Axboe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cfq-iosched: requests "in flight" vs "in driver" clarification
2010-02-27 16:03 [PATCH] cfq-iosched: requests "in flight" vs "in driver" clarification Corrado Zoccolo
2010-02-28 18:44 ` Jens Axboe
@ 2010-03-01 14:11 ` Vivek Goyal
1 sibling, 0 replies; 4+ messages in thread
From: Vivek Goyal @ 2010-03-01 14:11 UTC (permalink / raw)
To: Corrado Zoccolo
Cc: Jens Axboe, Linux-Kernel, Jeff Moyer, Shaohua Li, Gui Jianfeng
On Sat, Feb 27, 2010 at 05:03:09PM +0100, Corrado Zoccolo wrote:
> Counters for requests "in flight" and "in driver" are used asymmetrically
> in cfq_may_dispatch, and have slightly different meaning.
> We split the rq_in_flight counter (was sync_flight) to count both sync
> and async requests, in order to use this one, which is more accurate in
> some corner cases.
> The rq_in_driver counter is coalesced, since individual sync/async counts
> are not used any more.
>
> Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
> ---
> block/cfq-iosched.c | 44 ++++++++++++++++++--------------------------
> 1 files changed, 18 insertions(+), 26 deletions(-)
>
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 10eb286..eed649c 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -223,8 +223,8 @@ struct cfq_data {
>
> unsigned int busy_queues;
>
> - int rq_in_driver[2];
> - int sync_flight;
> + int rq_in_driver;
> + int rq_in_flight[2];
>
> /*
> * queue-depth detection
> @@ -417,11 +417,6 @@ static struct cfq_queue *cfq_get_queue(struct cfq_data *, bool,
> static struct cfq_io_context *cfq_cic_lookup(struct cfq_data *,
> struct io_context *);
>
> -static inline int rq_in_driver(struct cfq_data *cfqd)
> -{
> - return cfqd->rq_in_driver[0] + cfqd->rq_in_driver[1];
> -}
> -
> static inline struct cfq_queue *cic_to_cfqq(struct cfq_io_context *cic,
> bool is_sync)
> {
> @@ -1415,9 +1410,9 @@ static void cfq_activate_request(struct request_queue *q, struct request *rq)
> {
> struct cfq_data *cfqd = q->elevator->elevator_data;
>
> - cfqd->rq_in_driver[rq_is_sync(rq)]++;
> + cfqd->rq_in_driver++;
> cfq_log_cfqq(cfqd, RQ_CFQQ(rq), "activate rq, drv=%d",
> - rq_in_driver(cfqd));
> + cfqd->rq_in_driver);
>
> cfqd->last_position = blk_rq_pos(rq) + blk_rq_sectors(rq);
> }
> @@ -1425,12 +1420,11 @@ static void cfq_activate_request(struct request_queue *q, struct request *rq)
> static void cfq_deactivate_request(struct request_queue *q, struct request *rq)
> {
> struct cfq_data *cfqd = q->elevator->elevator_data;
> - const int sync = rq_is_sync(rq);
>
> - WARN_ON(!cfqd->rq_in_driver[sync]);
> - cfqd->rq_in_driver[sync]--;
> + WARN_ON(!cfqd->rq_in_driver);
> + cfqd->rq_in_driver--;
> cfq_log_cfqq(cfqd, RQ_CFQQ(rq), "deactivate rq, drv=%d",
> - rq_in_driver(cfqd));
> + cfqd->rq_in_driver);
> }
>
> static void cfq_remove_request(struct request *rq)
> @@ -1873,8 +1867,7 @@ static void cfq_dispatch_insert(struct request_queue *q, struct request *rq)
> cfqq->dispatched++;
> elv_dispatch_sort(q, rq);
>
> - if (cfq_cfqq_sync(cfqq))
> - cfqd->sync_flight++;
> + cfqd->rq_in_flight[cfq_cfqq_sync(cfqq)]++;
> cfqq->nr_sectors += blk_rq_sectors(rq);
> }
>
> @@ -2221,13 +2214,13 @@ static bool cfq_may_dispatch(struct cfq_data *cfqd, struct cfq_queue *cfqq)
> /*
> * Drain async requests before we start sync IO
> */
> - if (cfq_should_idle(cfqd, cfqq) && cfqd->rq_in_driver[BLK_RW_ASYNC])
> + if (cfq_should_idle(cfqd, cfqq) && cfqd->rq_in_flight[BLK_RW_ASYNC])
> return false;
>
So we need to make sure there are no async requests either in dispatch
queue or driver. So rq_in_flight is better than just plain rq_in_driver.
Makes sense.
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Thanks
Vivek
> /*
> * If this is an async queue and we have sync IO in flight, let it wait
> */
> - if (cfqd->sync_flight && !cfq_cfqq_sync(cfqq))
> + if (cfqd->rq_in_flight[BLK_RW_SYNC] && !cfq_cfqq_sync(cfqq))
> return false;
>
> max_dispatch = cfqd->cfq_quantum;
> @@ -3210,14 +3203,14 @@ static void cfq_update_hw_tag(struct cfq_data *cfqd)
> {
> struct cfq_queue *cfqq = cfqd->active_queue;
>
> - if (rq_in_driver(cfqd) > cfqd->hw_tag_est_depth)
> - cfqd->hw_tag_est_depth = rq_in_driver(cfqd);
> + if (cfqd->rq_in_driver > cfqd->hw_tag_est_depth)
> + cfqd->hw_tag_est_depth = cfqd->rq_in_driver;
>
> if (cfqd->hw_tag == 1)
> return;
>
> if (cfqd->rq_queued <= CFQ_HW_QUEUE_MIN &&
> - rq_in_driver(cfqd) <= CFQ_HW_QUEUE_MIN)
> + cfqd->rq_in_driver <= CFQ_HW_QUEUE_MIN)
> return;
>
> /*
> @@ -3227,7 +3220,7 @@ static void cfq_update_hw_tag(struct cfq_data *cfqd)
> */
> if (cfqq && cfq_cfqq_idle_window(cfqq) &&
> cfqq->dispatched + cfqq->queued[0] + cfqq->queued[1] <
> - CFQ_HW_QUEUE_MIN && rq_in_driver(cfqd) < CFQ_HW_QUEUE_MIN)
> + CFQ_HW_QUEUE_MIN && cfqd->rq_in_driver < CFQ_HW_QUEUE_MIN)
> return;
>
> if (cfqd->hw_tag_samples++ < 50)
> @@ -3280,13 +3273,12 @@ static void cfq_completed_request(struct request_queue *q, struct request *rq)
>
> cfq_update_hw_tag(cfqd);
>
> - WARN_ON(!cfqd->rq_in_driver[sync]);
> + WARN_ON(!cfqd->rq_in_driver);
> WARN_ON(!cfqq->dispatched);
> - cfqd->rq_in_driver[sync]--;
> + cfqd->rq_in_driver--;
> cfqq->dispatched--;
>
> - if (cfq_cfqq_sync(cfqq))
> - cfqd->sync_flight--;
> + cfqd->rq_in_flight[cfq_cfqq_sync(cfqq)]--;
>
> if (sync) {
> RQ_CIC(rq)->last_end_request = now;
> @@ -3340,7 +3332,7 @@ static void cfq_completed_request(struct request_queue *q, struct request *rq)
> }
> }
>
> - if (!rq_in_driver(cfqd))
> + if (!cfqd->rq_in_driver)
> cfq_schedule_dispatch(cfqd);
> }
>
> --
> 1.6.4.4
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-03-01 14:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-27 16:03 [PATCH] cfq-iosched: requests "in flight" vs "in driver" clarification Corrado Zoccolo
2010-02-28 18:44 ` Jens Axboe
2010-03-01 14:11 ` Vivek Goyal
-- strict thread matches above, loose matches on Subject: below --
2009-12-30 21:16 [PATCH] cfq-iosched: replace sync_flight by rq_in_driver[BLK_RW_SYNC] Jens Axboe
2009-12-30 22:24 ` [PATCH] cfq-iosched: requests "in flight" vs "in driver" clarification Corrado Zoccolo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox