Patch from Nick Piggin This patch fixes my hack which kept the next_arq cache up to date via the rbtree add and delete functions. It also fixes a bug where we would assign the current_id (anticipating) with the new value _before_ we needed to use the old value. This would have caused as_choose_request not make descisions correctly. This changes behaviour a bit and I have not yet tested it. Should be alright. It also moves and comments as_choose_request a bit - it isn't an A.S. function. drivers/block/as-iosched.c | 227 +++++++++++++++++++++------------------- drivers/block/as-iosched.c.orig | 0 2 files changed, 120 insertions(+), 107 deletions(-) diff -puN drivers/block/as-iosched.c~as-cleanup-3 drivers/block/as-iosched.c --- 25/drivers/block/as-iosched.c~as-cleanup-3 2003-02-25 01:13:16.000000000 -0800 +++ 25-akpm/drivers/block/as-iosched.c 2003-02-25 01:13:16.000000000 -0800 @@ -273,7 +273,6 @@ static struct as_rq *__as_add_arq_rb(str } static void as_move_to_dispatch(struct as_data *ad, struct as_rq *arq); -static void as_update_arq(struct as_data *ad, struct as_rq *arq); /* * Aad the request to the rb tree if it is unique. If there is an alias (an @@ -292,36 +291,10 @@ static void as_add_arq_rb(struct as_data as_move_to_dispatch(ad, alias); rb_insert_color(&arq->rb_node, ARQ_RB_ROOT(ad, arq)); - as_update_arq(ad, arq); } -static struct as_rq * -as_choose_req(struct as_data *ad, struct as_rq *arq1, struct as_rq *arq2); - -static inline void -as_del_arq_rb(struct as_data *ad, struct as_rq *arq) +static inline void as_del_arq_rb(struct as_data *ad, struct as_rq *arq) { - const int data_dir = rq_data_dir(arq->request); - - if (ad->next_arq[data_dir] == arq) { - struct rb_node *rbnext = rb_next(&arq->rb_node); - struct rb_node *rbprev = rb_prev(&arq->rb_node); - struct as_rq *arq_next, *arq_prev; - - if (rbprev) - arq_prev = rb_entry_arq(rbprev); - else - arq_prev = NULL; - - if (rbnext) - arq_next = rb_entry_arq(rbnext); - else - arq_next = as_find_first_arq(ad, data_dir); - - ad->next_arq[data_dir] = as_choose_req(ad, - arq_next, arq_prev); - } - if (ON_RB(&arq->rb_node)) { rb_erase(&arq->rb_node, ARQ_RB_ROOT(ad, arq)); RB_CLEAR(&arq->rb_node); @@ -348,6 +321,8 @@ as_find_arq_rb(struct as_data *ad, secto return NULL; } +static void as_update_arq(struct as_data *ad, struct as_rq *arq); + /* * add arq to rbtree and fifo */ @@ -358,6 +333,9 @@ static void as_add_request(struct as_dat arq->request_id = request_id(); as_add_arq_rb(ad, arq); + + as_update_arq(ad, arq); /* keep state machine up to date */ + /* * set expire time (only used for reads) and add to fifo list */ @@ -365,12 +343,16 @@ static void as_add_request(struct as_dat list_add_tail(&arq->fifo, &ad->fifo_list[data_dir]); } +static struct as_rq * +as_choose_req(struct as_data *ad, struct as_rq *arq1, struct as_rq *arq2); + /* * remove rq from rbtree, fifo, and hash */ static void as_remove_request(request_queue_t *q, struct request *rq) { struct as_rq *arq = RQ_DATA(rq); + const int data_dir = rq_data_dir(arq->request); if (arq) { struct as_data *ad = q->elevator.elevator_data; @@ -378,6 +360,29 @@ static void as_remove_request(request_qu list_del_init(&arq->fifo); as_del_arq_hash(arq); as_del_arq_rb(ad, arq); + + /* + * Update the "next_arq" cache as we are about to remove its + * entry + */ + if (ad->next_arq[data_dir] == arq) { + struct rb_node *rbnext = rb_next(&arq->rb_node); + struct rb_node *rbprev = rb_prev(&arq->rb_node); + struct as_rq *arq_next, *arq_prev; + + if (rbprev) + arq_prev = rb_entry_arq(rbprev); + else + arq_prev = NULL; + + if (rbnext) + arq_next = rb_entry_arq(rbnext); + else + arq_next = as_find_first_arq(ad, data_dir); + + ad->next_arq[data_dir] = as_choose_req(ad, + arq_next, arq_prev); + } } if (q->last_merge == &rq->queuelist) @@ -457,6 +462,11 @@ static void as_merged_request(request_qu if (rq_rb_key(req) != arq->rb_key) { as_del_arq_rb(ad, arq); as_add_arq_rb(ad, arq); + /* + * Note! At this stage of this and the next function, our next + * request may not be optimal - eg the request may have "grown" + * behind the disk head. We currently don't bother adjusting. + */ } q->last_merge = &req->queuelist; @@ -518,7 +528,7 @@ static void as_move_to_dispatch(struct a if (data_dir == READ) /* In case we have to anticipate after this */ ad->current_id = arq->request_id; - + /* * take it off the sort and fifo list, move * to dispatch queue @@ -678,83 +688,6 @@ as_close_req(struct as_data *ad, struct return (last <= next) && (next <= last + delta); } -#define MAXBACK (512 * 1024) - -static struct as_rq * -as_choose_req(struct as_data *ad, struct as_rq *arq1, struct as_rq *arq2) -{ - int data_dir; - sector_t last, s1, s2, d1, d2; - const sector_t maxback = MAXBACK; - - if (arq1 == NULL || arq1 == arq2) - return arq2; - if (arq2 == NULL) - return arq1; - - data_dir = rq_data_dir(arq1->request); - last = ad->last_sector[data_dir]; - s1 = arq1->request->sector; - s2 = arq2->request->sector; - - BUG_ON(data_dir != rq_data_dir(arq2->request)); - - /* - * Strict one way elevator _except_ in the case where we allow - * short backward seeks which are biased as twice the cost of a - * similar forward seek. Only for reads and only between reads - * from the same process! - */ - if (s1 >= last) - d1 = s1 - last; - else if (data_dir == READ - && ad->current_id == arq1->request_id - && s1+maxback >= last) - d1 = (last - s1)*2; - else - goto elevator_wrap; - - if (s2 >= last) - d2 = s2 - last; - else if (data_dir == READ - && ad->current_id == arq2->request_id - && s2+maxback >= last) - d2 = (last - s2)*2; - else - goto elevator_wrap; - - /* Found the deltas */ - if (d1 < d2) - return arq1; - else if (d2 < d1) - return arq2; - else { - if (s1 >= s2) - return arq1; - else - return arq2; - } - -elevator_wrap: - /* - * a request is behind the disk head, in most - * cases this is treated as the elevator wrapping - * to the start, so a lower valued request (further away) - * is favourable - */ - if (s1 >= last && s2 < last) - return arq1; - else if (s2 >= last && s1 < last) - return arq2; - else { - /* both behind the head */ - if (s1 <= s2) - return arq1; - else - return arq2; - } -} - /* * as_can_break_anticipation returns true if we have been anticipating this * request. @@ -872,6 +805,87 @@ static int as_can_anticipate(struct as_d return 1; } +#define MAXBACK (512 * 1024) + +/* + * as_choose_req selects the preferred one of two requests of the same data_dir + * ignoring time - eg. timeouts, which is the job of as_dispatch_request + */ +static struct as_rq * +as_choose_req(struct as_data *ad, struct as_rq *arq1, struct as_rq *arq2) +{ + int data_dir; + sector_t last, s1, s2, d1, d2; + const sector_t maxback = MAXBACK; + + if (arq1 == NULL || arq1 == arq2) + return arq2; + if (arq2 == NULL) + return arq1; + + data_dir = rq_data_dir(arq1->request); + last = ad->last_sector[data_dir]; + s1 = arq1->request->sector; + s2 = arq2->request->sector; + + BUG_ON(data_dir != rq_data_dir(arq2->request)); + + /* + * Strict one way elevator _except_ in the case where we allow + * short backward seeks which are biased as twice the cost of a + * similar forward seek. Only for reads and only between reads + * from the same process! + */ + if (s1 >= last) + d1 = s1 - last; + else if (data_dir == READ + && ad->current_id == arq1->request_id + && s1+maxback >= last) + d1 = (last - s1)*2; + else + goto elevator_wrap; + + if (s2 >= last) + d2 = s2 - last; + else if (data_dir == READ + && ad->current_id == arq2->request_id + && s2+maxback >= last) + d2 = (last - s2)*2; + else + goto elevator_wrap; + + /* Found the deltas */ + if (d1 < d2) + return arq1; + else if (d2 < d1) + return arq2; + else { + if (s1 >= s2) + return arq1; + else + return arq2; + } + +elevator_wrap: + /* + * a request is behind the disk head, in most + * cases this is treated as the elevator wrapping + * to the start, so a lower valued request (further away) + * is favourable + */ + if (s1 >= last && s2 < last) + return arq1; + else if (s2 >= last && s1 < last) + return arq2; + else { + /* both behind the head */ + if (s1 <= s2) + return arq1; + else + return arq2; + } +} + /* * as_dispatch_request selects the best request according to * read/write expire, batch expire, etc, and moves it to the dispatch @@ -908,7 +922,6 @@ static int as_dispatch_request(struct as return 0; } - } if (arq) { diff -puN drivers/block/as-iosched.c.orig~as-cleanup-3 drivers/block/as-iosched.c.orig _