Consolidate the request merging
PATCH 1/1 Consolidate the request merging
Hi,
Right now, every IO scheduler implements its own backmerging (except for noop, which does no merging). That results in duplicated code for essentially the same operation, which is never a good thing. This patch moves the backmerging out of the io schedulers and into the elevator core. We save 1.6kb of text and as a bonus get backmerging for noop as well. Win-win!
Notes:
- I dropped the "move hot entries to front" logic. It's never been proven good, and some research indicates that it's a bad idea. I doubt it matters in real life, so lets just cut that away.
- Next it might be a good idea to move the rb sorting into the elevator core as well. We could save some more kernel text, but more importantly it gets us one step closer to dropping deadline_rq from the deadline scheduler.
Please gives this patch a test spin!
block/as-iosched.c | 125 --------------------------------------------- block/cfq-iosched.c | 66 ------------------------ block/deadline-iosched.c | 128 ----------------------------------------------- block/elevator.c | 101 +++++++++++++++++++++++++++++++++++-- block/ll_rw_blk.c | 2 include/linux/blkdev.h | 2 include/linux/elevator.h | 1 7 files changed, 105 insertions(+), 320 deletions(-)
block/as-iosched.c +0 -125
@@ -14,7 +14,6 @@ #include <linux/module.h>#include <linux/slab.h> #include <linux/init.h> #include <linux/compiler.h> -#include <linux/hash.h> #include <linux/rbtree.h> #include <linux/interrupt.h>
@@ -95,7 +94,6 @@ struct as_data {struct as_rq *next_arq[2 ];/* next in sort order */ sector_t last_sector[2 ];/* last REQ_SYNC & REQ_ASYNC sectors */ -struct hlist_head *hash;/* request hash */ unsigned long exit_prob;/* probability a task will exit while being waited on */
@@ -162,11 +160,6 @@ struct as_rq {struct io_context *io_context;/* The submitting task */ /* -* request hash, key is the ending offset (for back merge lookup) -*/ -struct hlist_node hash; - -/* * expire fifo */ struct list_head fifo;
@@ -273,77 +266,6 @@ static void as_put_io_context(struct as_ }/* -* the back merge hash support functions -*/ -static const int as_hash_shift =6 ; -#define AS_HASH_BLOCK (sec) ((sec) >> 3) -#define AS_HASH_FN (sec) (hash_long(AS_HASH_BLOCK((sec)), as_hash_shift)) -#define AS_HASH_ENTRIES (1 << as_hash_shift) -#define rq_hash_key (rq) ((rq)->sector + (rq)->nr_sectors) - -static inline void __as_del_arq_hash (struct as_rq *arq) -{ -hlist_del_init (&arq->hash); -} - -static inline void as_del_arq_hash (struct as_rq *arq) -{ -if (!hlist_unhashed (&arq->hash)) -__as_del_arq_hash (arq); -} - -static void as_add_arq_hash (struct as_data *ad,struct as_rq *arq) -{ -struct request *rq = arq->request; - -BUG_ON (!hlist_unhashed (&arq->hash)); - -hlist_add_head (&arq->hash, &ad->hash[AS_HASH_FN (rq_hash_key (rq))]); -} - -/* -* move hot entry to front of chain -*/ -static inline void as_hot_arq_hash (struct as_data *ad,struct as_rq *arq) -{ -struct request *rq = arq->request; -struct hlist_head *head = &ad->hash[AS_HASH_FN (rq_hash_key (rq))]; - -if (hlist_unhashed (&arq->hash)) { -WARN_ON (1 ); -return ; - } - -if (&arq->hash != head->first) { -hlist_del (&arq->hash); -hlist_add_head (&arq->hash, head); - } -} - -static struct request *as_find_arq_hash (struct as_data *ad,sector_t offset) -{ -struct hlist_head *hash_list = &ad->hash[AS_HASH_FN (offset)]; -struct hlist_node *entry, *next; -struct as_rq *arq; - -hlist_for_each_entry_safe (arq, entry, next, hash_list, hash) { -struct request *__rq = arq->request; - -BUG_ON (hlist_unhashed (&arq->hash)); - -if (!rq_mergeable (__rq)) { -as_del_arq_hash (arq); -continue ; - } - -if (rq_hash_key (__rq) == offset) -return __rq; - } - -return NULL ; -} - -/* * rb tree support functions */ #define rb_entry_arq (node) rb_entry((node), struct as_rq, rb_node)
@@ -1060,7 +982,6 @@ static void as_remove_queued_request(req ad->next_arq[data_dir] =as_find_next_arq (ad, arq);list_del_init (&arq->fifo); -as_del_arq_hash (arq);as_del_arq_rb (ad, arq); }
@@ -1349,8 +1270,6 @@ static void as_add_request(request_queue }as_add_arq_rb (ad, arq); -if (rq_mergeable (arq->request)) -as_add_arq_hash (ad, arq);/* * set expire time (only used for reads) and add to fifo list
@@ -1431,19 +1350,6 @@ as_merge(request_queue_t *q, struct requint ret;/* -* see if the merge hash can satisfy a back merge -*/ - __rq =as_find_arq_hash (ad, bio->bi_sector); -if (__rq) { -BUG_ON (__rq->sector + __rq->nr_sectors != bio->bi_sector); - -if (elv_rq_merge_ok (__rq, bio)) { - ret =ELEVATOR_BACK_MERGE ; - goto out; - } - } - -/* * check for front merge */ __rq =as_find_arq_rb (ad, rb_key,bio_data_dir (bio));
@@ -1458,10 +1364,6 @@ as_merge(request_queue_t *q, struct requreturn ELEVATOR_NO_MERGE ; out: -if (ret) { -if (rq_mergeable (__rq)) -as_hot_arq_hash (ad,RQ_DATA (__rq)); - } *req = __rq;return ret; }
@@ -1472,12 +1374,6 @@ static void as_merged_request(request_qustruct as_rq *arq =RQ_DATA (req);/* -* hash always needs to be repositioned, key is end sector -*/ -as_del_arq_hash (arq); -as_add_arq_hash (ad, arq); - -/* * if the merge was a front merge, we need to reposition request */ if (rq_rb_key (req) != arq->rb_key) {
@@ -1501,13 +1397,6 @@ static void as_merged_requests(request_qBUG_ON (!arq);BUG_ON (!anext); -/* -* reposition arq (this is the merged request) in hash, and in rbtree -* in case of a front merge -*/ -as_del_arq_hash (arq); -as_add_arq_hash (ad, arq); -if (rq_rb_key (req) != arq->rb_key) {as_del_arq_rb (ad, arq);as_add_arq_rb (ad, arq);
@@ -1591,7 +1480,6 @@ static int as_set_request(request_queue_ arq->request = rq; arq->state =AS_RQ_PRESCHED ; arq->io_context =NULL ; -INIT_HLIST_NODE (&arq->hash);INIT_LIST_HEAD (&arq->fifo); rq->elevator_private = arq;return 0 ;
@@ -1628,7 +1516,6 @@ static void as_exit_queue(elevator_t *e)mempool_destroy (ad->arq_pool);put_io_context (ad->io_context); -kfree (ad->hash);kfree (ad); }
@@ -1639,7 +1526,6 @@ static void as_exit_queue(elevator_t *e)static void *as_init_queue (request_queue_t *q,elevator_t *e) {struct as_data *ad; -int i;if (!arq_pool)return NULL ;
@@ -1651,17 +1537,9 @@ static void *as_init_queue(request_queue ad->q = q;/* Identify what queue the data belongs to */ - ad->hash =kmalloc_node (sizeof (struct hlist_head )*AS_HASH_ENTRIES , -GFP_KERNEL , q->node); -if (!ad->hash) { -kfree (ad); -return NULL ; - } - ad->arq_pool =mempool_create_node (BLKDEV_MIN_RQ , mempool_alloc_slab, mempool_free_slab, arq_pool, q->node);if (!ad->arq_pool) { -kfree (ad->hash);kfree (ad);return NULL ; }
@@ -1672,9 +1550,6 @@ static void *as_init_queue(request_queueinit_timer (&ad->antic_timer);INIT_WORK (&ad->antic_work, as_work_handler, q); -for (i =0 ; i <AS_HASH_ENTRIES ; i++) -INIT_HLIST_HEAD (&ad->hash[i]); -INIT_LIST_HEAD (&ad->fifo_list[REQ_SYNC ]);INIT_LIST_HEAD (&ad->fifo_list[REQ_ASYNC ]); ad->sort_list[REQ_SYNC ] =RB_ROOT ;
block/cfq-iosched.c +1 -65
129 lines not on this page · raw message
block/deadline-iosched.c +2 -126
236 lines not on this page · raw message
block/elevator.c +97 -4
182 lines not on this page · raw message
block/ll_rw_blk.c +2 -0
@@ -281,6 +281,7 @@ static inline void rq_init(request_queue {INIT_LIST_HEAD (&rq->queuelist);INIT_LIST_HEAD (&rq->donelist); +INIT_HLIST_NODE (&rq->hash); rq->errors =0 ; rq->rq_status =RQ_ACTIVE ;
@@ -2665,6 +2666,7 @@ void __blk_put_request(request_queue_t *int priv = req->flags &REQ_ELVPRIV ;BUG_ON (!list_empty (&req->queuelist)); +BUG_ON (!hlist_unhashed (&req->hash));blk_free_request (q, req);freed_request (q, rw, priv);
include/linux/blkdev.h +2 -0
@@ -148,6 +148,8 @@ struct request {struct bio *bio;struct bio *biotail; +struct hlist_node hash;/* merge hash */ +void *elevator_private;void *completion_data;
include/linux/elevator.h +1 -0
@@ -82,6 +82,7 @@ struct elevator_queuestruct kobject kobj;struct elevator_type *elevator_type;struct mutex sysfs_lock; +struct hlist_head *hash; };/*
Signature · 1 line
Jens Axboe
Quoted 11 lines · Jens Axboe
Hi,
Right now, every IO scheduler implements its own backmerging (except for noop, which does no merging). That results in duplicated code for essentially the same operation, which is never a good thing. This patch moves the backmerging out of the io schedulers and into the elevator core. We save 1.6kb of text and as a bonus get backmerging for noop as well. Win-win!
Notes:
Quoted 8 lines
- I dropped the "move hot entries to front" logic. It's never been proven good, and some research indicates that it's a bad idea. I doubt it matters in real life, so lets just cut that away.
- Next it might be a good idea to move the rb sorting into the elevator core as well. We could save some more kernel text, but more importantly it gets us one step closer to dropping
deadline_rqfrom the deadline scheduler.
Seems like a good idea. I don't think this could be a downside for anyone except maybe Ken Chen, if it adds any overhead to the noop scheduler.
BTW, IMO it is a good idea for the noop scheduler to do as much merging as possible, especially as it could be used for things like network block devices (but more merging may actually cut down on CPU and IO bandwidth even in the local disk case).
Signature · 2 lines
SUSE Labs, Novell Inc. Send instant messages to your online friends http://au.messenger.yahoo.com
Quoted 21 lines · Nick Piggin
Quoted 11 lines · Jens Axboe
Hi,
Right now, every IO scheduler implements its own backmerging (except for noop, which does no merging). That results in duplicated code for essentially the same operation, which is never a good thing. This patch moves the backmerging out of the io schedulers and into the elevator core. We save 1.6kb of text and as a bonus get backmerging for noop as well. Win-win!
Notes:
Quoted 8 lines
- I dropped the "move hot entries to front" logic. It's never been proven good, and some research indicates that it's a bad idea. I doubt it matters in real life, so lets just cut that away.
- Next it might be a good idea to move the rb sorting into the elevator core as well. We could save some more kernel text, but more importantly it gets us one step closer to dropping
deadline_rqfrom the deadline scheduler.
Quoted 7 lines
Seems like a good idea. I don't think this could be a downside for anyone except maybe Ken Chen, if it adds any overhead to the noop scheduler.
BTW, IMO it is a good idea for the noop scheduler to do as much merging as possible, especially as it could be used for things like network block devices (but more merging may actually cut down on CPU and IO bandwidth even in the local disk case).
I agree, I actually think it's a win for noop with the cheap merging. If anyone complains, we can always make it tweakable with a sysfs parameter. Even for "intelligent" hardware, you have a command overhead per request, so it makes a lot of sense to always do merging.
Signature · 1 line
Jens Axboe