Re: [PATCH] 2.5.28 small REQ_SPECIAL abstraction

From: Jens Axboe (axboe@suse.de)
Date: Fri Jul 26 2002 - 09:38:40 EST

On Fri, Jul 26 2002, Marcin Dalecki wrote:
> The attached patch does the following:

Looks fine to me. One thing sticks out though:

> + /*
> + * tell I/O scheduler that this isn't a regular read/write (ie it
> + * must not attempt merges on this) and that it acts as a soft
> + * barrier
> + */
> + rq->flags &= REQ_QUEUED;

this can't be right. Either it's a bug for REQ_QUEUED to be set here, or
it needs to end the tag properly.

> + rq->flags |= REQ_SPECIAL | REQ_BARRIER;
> +
> + rq->special = data;
> +
> + spin_lock_irqsave(q->queue_lock, flags);
> + /* If command is tagged, release the tag */
> + if(blk_rq_tagged(rq))
> + blk_queue_end_tag(q, rq);

woops, you just possible leaked a tag. I really don't think you should
mix inserting a special and ending a tag into the same function, makes
no sense. If a driver wants that it should do:

        if (blk_rq_tagged(rq))
                blk_queue_end_tag(q, rq);

        blk_insert_request(q, rq, bla bla);

Also, please use the right spacing, if(bla :-)

So kill any reference to tagging (and REQ_QUEUED)i in
blk_insert_request, and I'm ok with it. 

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

This archive was generated by hypermail 2b29 : Tue Jul 30 2002 - 14:00:24 EST