Re: [PATCH] make nbd working in 2.5.x
Petr Vandrovec (VANDROVE@vc.cvut.cz)
Wed, 5 Mar 2003 12:37:21 +0100
On 5 Mar 03 at 10:21, Jens Axboe wrote:
> On Mon, Mar 03 2003, Petr Vandrovec wrote:
> > On 3 Mar 03 at 19:39, Pavel Machek wrote:
> >
> > I think that at the beginning of 2.5.x series there was some thinking
> > about removing end_that_request* completely from the API. As it never
> > happened, and __end_that_request_first()/end_that_request_last() has
> > definitely better quality (like that it does not ignore req->waiting...)
> > than opencoded nbd loop, I prefer using end_that_request* over opencoding
> > bio traversal.
> >
> > If you want, then just replace blk_put_request() with __blk_put_request(),
> > instead of first change. But I personally will not trust such code, as
> > next time something in bio changes nbd will miss this change again.
>
> I agree with the change, there's no reason for nbd to implement its own
> end_request handling. I was the one to do the bio conversion, doing XXX
> drivers at one time...
>
> A small correction to your patch, you need not hold queue_lock when
> calling end_that_request_first() (which is the costly part of ending a
> request), so
>
> if (!end_that_request_first(req, uptodate, req->nr_sectors)) {
> unsigned long flags;
>
> spin_lock_irqsave(q->queue_lock, flags);
> end_that_request_last(req);
> spin_unlock_irqrestore(q->queue_lock, flags);
> }
>
> would be enough. That depends on the driver having pulled the request
> off the list in the first place, which nbd has.
But it also finishes whole request at once, so probably with:
if (!end_that_request_first(...)) {
...
} else {
BUG();
}
I had patch for 2.5.3 which finished request partially after each chunk
(usually 1500 bytes) received from server, but it did not make any
difference in performance at that time (probably because of the way
nbd server works and speed of network between server and client). I'll
try it now again...
Petr Vandrovec
vandrove@vc.cvut.cz
-
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/