All of lore.kernel.org
 help / color / mirror / Atom feed
From: Corrado Zoccolo <czoccolo@gmail.com>
To: Jens Axboe <jens.axboe@oracle.com>
Cc: Linux-Kernel <linux-kernel@vger.kernel.org>,
	Jeff Moyer <jmoyer@redhat.com>, Vivek Goyal <vgoyal@redhat.com>,
	Shaohua Li <shaohua.li@intel.com>,
	Gui Jianfeng <guijianfeng@cn.fujitsu.com>,
	Corrado Zoccolo <czoccolo@gmail.com>
Subject: [PATCH] cfq-iosched: NCQ SSDs do not need read queue merging
Date: Sun, 10 Jan 2010 22:08:04 +0100	[thread overview]
Message-ID: <1263157684-13157-1-git-send-email-czoccolo@gmail.com> (raw)
In-Reply-To: <1263157461-12294-1-git-send-email-czoccolo@gmail.com>

NCQ SSDs' performances are not affected by
distance of read requests, so there is no point in having
overhead to merge such queues.

Non-NCQ SSDs showed regression in some special cases, so
they are ruled out by this patch.

This patch intentionally doesn't affect writes, so
it changes the queued[] field, to be indexed by
READ/WRITE instead of SYNC/ASYNC, and only compute proximity
for queues with WRITE requests.

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

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 918c7fd..18bce18 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -108,9 +108,9 @@ struct cfq_queue {
 	struct rb_root sort_list;
 	/* if fifo isn't expired, next request to serve */
 	struct request *next_rq;
-	/* requests queued in sort_list */
+	/* requests queued in sort_list, indexed by READ/WRITE */
 	int queued[2];
-	/* currently allocated requests */
+	/* currently allocated requests, indexed by READ/WRITE */
 	int allocated[2];
 	/* fifo list of requests in sort_list */
 	struct list_head fifo;
@@ -436,6 +436,10 @@ static inline void cic_set_cfqq(struct cfq_io_context *cic,
 	cic->cfqq[is_sync] = cfqq;
 }
 
+static inline bool is_smart_ssd(struct cfq_data *cfqd)
+{
+	return blk_queue_nonrot(cfqd->queue) && cfqd->hw_tag;
+}
 /*
  * We regard a request as SYNC, if it's either a read or has the SYNC bit
  * set (in which case it could also be direct WRITE).
@@ -1268,7 +1272,8 @@ static void cfq_prio_tree_add(struct cfq_data *cfqd, struct cfq_queue *cfqq)
 		return;
 	if (!cfqq->next_rq)
 		return;
-
+	if (is_smart_ssd(cfqd) && !cfqq->queued[WRITE])
+		return;
 	cfqq->p_root = &cfqd->prio_trees[cfqq->org_ioprio];
 	__cfqq = cfq_prio_tree_lookup(cfqd, cfqq->p_root,
 				      blk_rq_pos(cfqq->next_rq), &parent, &p);
@@ -1337,10 +1342,10 @@ static void cfq_del_cfqq_rr(struct cfq_data *cfqd, struct cfq_queue *cfqq)
 static void cfq_del_rq_rb(struct request *rq)
 {
 	struct cfq_queue *cfqq = RQ_CFQQ(rq);
-	const int sync = rq_is_sync(rq);
+	const int rw = rq_data_dir(rq);
 
-	BUG_ON(!cfqq->queued[sync]);
-	cfqq->queued[sync]--;
+	BUG_ON(!cfqq->queued[rw]);
+	cfqq->queued[rw]--;
 
 	elv_rb_del(&cfqq->sort_list, rq);
 
@@ -1363,7 +1368,7 @@ static void cfq_add_rq_rb(struct request *rq)
 	struct cfq_data *cfqd = cfqq->cfqd;
 	struct request *__alias, *prev;
 
-	cfqq->queued[rq_is_sync(rq)]++;
+	cfqq->queued[rq_data_dir(rq)]++;
 
 	/*
 	 * looks a little odd, but the first insert might return an alias.
@@ -1393,7 +1398,7 @@ static void cfq_add_rq_rb(struct request *rq)
 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)]--;
+	cfqq->queued[rq_data_dir(rq)]--;
 	cfq_add_rq_rb(rq);
 }
 
@@ -1689,7 +1694,8 @@ static struct cfq_queue *cfqq_close(struct cfq_data *cfqd,
 	struct cfq_queue *__cfqq;
 	sector_t sector = cfqd->last_position;
 
-	if (RB_EMPTY_ROOT(root))
+	if (RB_EMPTY_ROOT(root) ||
+	    (is_smart_ssd(cfqd) && !cur_cfqq->queued[WRITE]))
 		return NULL;
 
 	/*
@@ -1796,7 +1802,7 @@ static bool cfq_should_idle(struct cfq_data *cfqd, struct cfq_queue *cfqq)
 
 	/* We do for queues that were marked with idle window flag. */
 	if (cfq_cfqq_idle_window(cfqq) &&
-	   !(blk_queue_nonrot(cfqd->queue) && cfqd->hw_tag))
+	   !is_smart_ssd(cfqd))
 		return true;
 
 	/*
@@ -1817,7 +1823,7 @@ static void cfq_arm_slice_timer(struct cfq_data *cfqd)
 	 * for devices that support queuing, otherwise we still have a problem
 	 * with sync vs async workloads.
 	 */
-	if (blk_queue_nonrot(cfqd->queue) && cfqd->hw_tag)
+	if (is_smart_ssd(cfqd))
 		return;
 
 	WARN_ON(!RB_EMPTY_ROOT(&cfqq->sort_list));
-- 
1.6.4.4


  reply	other threads:[~2010-01-10 21:07 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-30 12:10 [PATCH] cfq-iosched: non-rot devices do not need queue merging Corrado Zoccolo
2009-12-30 18:45 ` Jens Axboe
2009-12-30 20:31   ` Corrado Zoccolo
2009-12-30 21:11     ` Jens Axboe
2009-12-30 21:21       ` Corrado Zoccolo
2009-12-30 21:34         ` Jens Axboe
2009-12-30 22:22           ` [PATCH] cfq-iosched: non-rot devices do not need read " Corrado Zoccolo
2010-01-04 14:47             ` Vivek Goyal
2010-01-04 16:36               ` Corrado Zoccolo
2010-01-04 16:51                 ` Jeff Moyer
2010-01-04 18:32                   ` Vivek Goyal
2010-01-04 18:37                   ` Corrado Zoccolo
2010-01-04 18:51                     ` Vivek Goyal
2010-01-04 19:04                       ` Jeff Moyer
2010-01-04 20:37                         ` Corrado Zoccolo
2010-01-05 14:58                           ` Jeff Moyer
2010-01-05 15:13                             ` Vivek Goyal
2010-01-05 21:19                               ` Jeff Moyer
2010-01-05 21:48                                 ` Corrado Zoccolo
2010-01-07 10:56                                   ` Kirill Afonshin
2010-01-07 13:38                                     ` Corrado Zoccolo
2010-01-07 14:36                                       ` Vivek Goyal
2010-01-07 17:00                                         ` Corrado Zoccolo
2010-01-07 18:37                                           ` Vivek Goyal
2010-01-07 20:16                                             ` Corrado Zoccolo
2010-01-08 18:53                                               ` Vivek Goyal
2010-01-10 12:55                                   ` Corrado Zoccolo
2010-01-10 21:04             ` [PATCH] cfq-iosched: NCQ SSDs " Corrado Zoccolo
2010-01-10 21:08               ` Corrado Zoccolo [this message]
2010-01-11 11:25               ` Jeff Garzik
2010-01-11 12:26                 ` Corrado Zoccolo
2010-01-11 13:13                   ` Jens Axboe
2010-01-11 13:18                     ` Jeff Garzik
2010-01-11 13:24                       ` Jens Axboe
2010-01-11 14:53                       ` Corrado Zoccolo
2010-01-11 16:44                         ` Vivek Goyal
2010-01-11 17:00                           ` Corrado Zoccolo
2010-01-11 17:07                             ` Vivek Goyal
2010-01-11 19:05                               ` Corrado Zoccolo
2010-01-11 17:11                             ` Vivek Goyal
2010-01-11 19:09                               ` Corrado Zoccolo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1263157684-13157-1-git-send-email-czoccolo@gmail.com \
    --to=czoccolo@gmail.com \
    --cc=guijianfeng@cn.fujitsu.com \
    --cc=jens.axboe@oracle.com \
    --cc=jmoyer@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shaohua.li@intel.com \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.