public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cfq-iosched: reduce write depth only if sync was delayed
@ 2009-12-04 12:35 Corrado Zoccolo
  2009-12-05 11:13 ` Corrado Zoccolo
  2009-12-08 18:00 ` Jeff Moyer
  0 siblings, 2 replies; 24+ messages in thread
From: Corrado Zoccolo @ 2009-12-04 12:35 UTC (permalink / raw)
  To: Linux-Kernel, Jens Axboe, Jeff Moyer, Vivek Goyal

The introduction of ramp-up formula for async queue depths has
slowed down dirty page reclaim, by reducing async write performance.
This patch makes sure the formula kicks in only when sync request
was recently delayed.

Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
---
 block/cfq-iosched.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index b00ca4c..a594388 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -284,7 +284,7 @@ struct cfq_data {
 	 */
 	struct cfq_queue oom_cfqq;
 
-	unsigned long last_end_sync_rq;
+	unsigned long last_delayed_sync;
 
 	/* List of cfq groups being managed on this device*/
 	struct hlist_head cfqg_list;
@@ -2264,7 +2264,7 @@ static bool cfq_may_dispatch(struct cfq_data *cfqd, 
struct cfq_queue *cfqq)
 	 * based on the last sync IO we serviced
 	 */
 	if (!cfq_cfqq_sync(cfqq) && cfqd->cfq_latency) {
-		unsigned long last_sync = jiffies - cfqd->last_end_sync_rq;
+		unsigned long last_sync = jiffies - cfqd->last_delayed_sync;
 		unsigned int depth;
 
 		depth = last_sync / cfqd->cfq_slice[1];
@@ -3272,7 +3272,8 @@ static void cfq_completed_request(struct request_queue 
*q, struct request *rq)
 
 	if (sync) {
 		RQ_CIC(rq)->last_end_request = now;
-		cfqd->last_end_sync_rq = now;
+		if (time_after(rq->start_time + cfqd->cfq_fifo_expire[1], now))
+			cfqd->last_delayed_sync = now;
 	}
 
 	/*
@@ -3706,7 +3707,7 @@ static void *cfq_init_queue(struct request_queue *q)
 	cfqd->cfq_latency = 1;
 	cfqd->cfq_group_isolation = 0;
 	cfqd->hw_tag = -1;
-	cfqd->last_end_sync_rq = jiffies;
+	cfqd->last_delayed_sync = jiffies - HZ;
 	return cfqd;
 }
 
-- 
1.6.2.5




^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH] cfq-iosched: reduce write depth only if sync was delayed
@ 2009-12-09 19:45 Corrado Zoccolo
  2009-12-09 19:51 ` Jeff Moyer
  2009-12-09 19:54 ` Jens Axboe
  0 siblings, 2 replies; 24+ messages in thread
From: Corrado Zoccolo @ 2009-12-09 19:45 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Jeff Moyer, Linux-Kernel, Vivek Goyal

On Wed, Dec 9, 2009 at 8:05 PM, Jens Axboe <jens.axboe@oracle.com> wrote:
> On Wed, Dec 09 2009, Jeff Moyer wrote:
>> OK.  Can we put a comment in there and change the initialization to
>> cfq_slice_sync * 10?
>
> Agree, that would be MUCH easier to understand.
>
Sure, we can put a comment there, but I don't like hardcoding a constant that depends on how the formula is computed (what if the formula is changed, 
and it doesn't depend on cfq_slice_sync any more, or if cfq_slice_sync changes dynamically?).
When I wrote it, what I really meant was exactly what you read in the C code (assume the last delayed sync happened 1 second ago). Then, the effect 
would be to start with a queue depth of 10 with the current formula, but even if we change the formula, 1 second is still meaningful (while 10 
*cfq_slice_sync, that has the same value, becomes misleading). So my proposed fix is just:

>From f06cd83b45b3a7ee13ae7322197b610085dc70dd Mon Sep 17 00:00:00 2001
From: Corrado Zoccolo <corrado@localhost.(none)>
Date: Wed, 9 Dec 2009 20:40:16 +0100
Subject: [PATCH] cfq-iosched: commenting non-obvious initialization

Added a comment to explain the initialization of last_delayed_sync.

Signed-off-by: Corrado Zoccolo <czoccolo@gmail.com>
---
 block/cfq-iosched.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 98b15b9..69ecee7 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -3759,6 +3759,10 @@ static void *cfq_init_queue(struct request_queue *q)
 	cfqd->cfq_latency = 1;
 	cfqd->cfq_group_isolation = 0;
 	cfqd->hw_tag = -1;
+	/*
+	 * we optimistically start assuming sync ops weren't delayed in last
+	 * second, in order to have larger depth for async operations.
+	 */
 	cfqd->last_delayed_sync = jiffies - HZ;
 	INIT_RCU_HEAD(&cfqd->rcu);
 	return cfqd;
-- 
1.6.2.5




^ permalink raw reply related	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2009-12-18 21:03 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-04 12:35 [PATCH] cfq-iosched: reduce write depth only if sync was delayed Corrado Zoccolo
2009-12-05 11:13 ` Corrado Zoccolo
2009-12-06 10:45   ` Corrado Zoccolo
2009-12-06 10:49     ` Jens Axboe
2009-12-07 14:13       ` Jeff Moyer
2009-12-07 14:41         ` Jens Axboe
2009-12-07 16:45           ` Jeff Moyer
2009-12-07 17:04             ` Vivek Goyal
2009-12-07 17:27             ` Vivek Goyal
2009-12-07 18:03               ` Vivek Goyal
2009-12-08  0:07     ` Jeff Moyer
2009-12-08 20:43       ` Corrado Zoccolo
2009-12-09 18:09         ` Jeff Moyer
2009-12-11 17:15           ` Corrado Zoccolo
2009-12-11 17:50             ` Jeff Moyer
2009-12-18 15:32             ` Jeff Moyer
2009-12-18 21:03               ` Corrado Zoccolo
2009-12-08 18:00 ` Jeff Moyer
2009-12-08 20:46   ` Corrado Zoccolo
2009-12-09 18:16     ` Jeff Moyer
2009-12-09 19:05       ` Jens Axboe
  -- strict thread matches above, loose matches on Subject: below --
2009-12-09 19:45 Corrado Zoccolo
2009-12-09 19:51 ` Jeff Moyer
2009-12-09 19:54 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox