On Wed, Oct 19 2005, Tejun Heo wrote:
> @@ -40,6 +40,11 @@
> static DEFINE_SPINLOCK(elv_list_lock);
> static LIST_HEAD(elv_list);
>
> +static inline sector_t rq_last_sector(struct request *rq)
> +{
> + return rq->sector + rq->nr_sectors;
> +}
Slightly misnamed, since it's really the sector after the last sector
:-)
I've renamed that to rq_end_sector() instead.
> +/*
> + * Insert rq into dispatch queue of q. Queue lock must be held on
> + * entry. If sort != 0, rq is sort-inserted; otherwise, rq will be
> + * appended to the dispatch queue. To be used by specific elevators.
> + */
> +void elv_dispatch_insert(request_queue_t *q, struct request *rq, int sort)
> +{
> + sector_t boundary;
> + unsigned max_back;
> + struct list_head *entry;
> +
> + if (!sort) {
> + /* Specific elevator is performing sort. Step away. */
> + q->last_sector = rq_last_sector(rq);
> + q->boundary_rq = rq;
> + list_add_tail(&rq->queuelist, &q->queue_head);
> + return;
> + }
> +
> + boundary = q->last_sector;
> + max_back = q->max_back_kb * 2;
> + boundary = boundary > max_back ? boundary - max_back : 0;
This looks really strange, what are you doing with boundary here?
> + list_for_each_prev(entry, &q->queue_head) {
> + struct request *pos = list_entry_rq(entry);
> +
> + if (pos->flags & (REQ_SOFTBARRIER|REQ_HARDBARRIER|REQ_STARTED))
> + break;
> + if (rq->sector >= boundary) {
> + if (pos->sector < boundary)
> + continue;
> + } else {
> + if (pos->sector >= boundary)
> + break;
> + }
> + if (rq->sector >= pos->sector)
> + break;
> + }
> +
> + list_add(&rq->queuelist, entry);
> +}
I've split this into, I don't like rolled-up functions that really do
two seperate things. So elv_dispatch_sort() now does sorting,
elv_dispatch_add_tail() does what !sort would have done.
> while ((rq = __elv_next_request(q)) != NULL) {
> - /*
> - * just mark as started even if we don't start it, a request
> - * that has been delayed should not be passed by new incoming
> - * requests
> - */
> - rq->flags |= REQ_STARTED;
> + if (!(rq->flags & REQ_STARTED)) {
> + elevator_t *e = q->elevator;
> +
> + /*
> + * This is the first time the device driver
> + * sees this request (possibly after
> + * requeueing). Notify IO scheduler.
> + */
> + if (blk_sorted_rq(rq) &&
> + e->ops->elevator_activate_req_fn)
> + e->ops->elevator_activate_req_fn(q, rq);
> +
> + /*
> + * just mark as started even if we don't start
> + * it, a request that has been delayed should
> + * not be passed by new incoming requests
> + */
> + rq->flags |= REQ_STARTED;
> + }
>
> if (rq == q->last_merge)
> q->last_merge = NULL;
>
> + if (!q->boundary_rq || q->boundary_rq == rq) {
> + q->last_sector = rq_last_sector(rq);
> + q->boundary_rq = NULL;
> + }
This seems to be the only place where you clear ->boundary_rq, that
can't be right. What about rq-to-rq merging, ->boundary_rq could be
freed and you wont notice. Generally I don't really like keeping
pointers to rqs around, it's given us problems in the past with the
last_merge bits even. For now I've added a clear of this in
__blk_put_request() as well.
> int elv_queue_empty(request_queue_t *q)
> {
> elevator_t *e = q->elevator;
>
> + if (!list_empty(&q->queue_head))
> + return 0;
> +
> if (e->ops->elevator_queue_empty_fn)
> return e->ops->elevator_queue_empty_fn(q);
>
> - return list_empty(&q->queue_head);
> + return 1;
> }
Agree, this order definitely makes more sense.
> @@ -2475,14 +2478,14 @@ static void __blk_put_request(request_qu
>
> void blk_put_request(struct request *req)
> {
> + unsigned long flags;
> + request_queue_t *q = req->q;
> +
> /*
> - * if req->rl isn't set, this request didnt originate from the
> - * block layer, so it's safe to just disregard it
> + * Gee, IDE calls in w/ NULL q. Fix IDE and remove the
> + * following if (q) test.
> */
> - if (req->rl) {
> - unsigned long flags;
> - request_queue_t *q = req->q;
> -
> + if (q) {
The q == NULL is because ide is using requests allocated on the stack,
I've wanted for that to die for many years :)
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
