Re: [PATCHSET 0/2] Don't let blk_put_request leak BIOs

[Jens Axboe <jens.axboe@oracle.com>]

On Thu, Mar 19 2009, Boaz Harrosh wrote:
> Hi Jens
> 
> You never commented on these patches. Please have a look?
> 
> The issue is that if we map some memory into a request but then
> do not execute it. Then calling blk_put_request() will leak the bio(s)
> unless one does an ugly code like:
> -	struct bio *bio;
> -
> -	while ((bio = rq->bio) != NULL) {
> -		rq->bio = bio->bi_next;
> -		bio_endio(bio, 0);
> -	}

Sorry, I think this is a horrible design. blk_put_request() doesn't care
about any data attachments, in fact (if possible) it should go BUG() if
the request hasn't been completed in some way or other. It deals with
the deallocation part, blk_get_request() doesn't attach any data. The
end result with code like the above is an assymmetric API.

> This problem arise in OSD when we can fail to setup the write
> or the read side and then we must cleanup the other half.
> Same problem exist in bsg, on bidi commands. But there the bio
> is just leaked on the error path, it does not do the ugly loop above.

These drivers should just be fixed, then.

So if you have spotted this problem in eg bsg, then please send a patch
for that to Tomo!

-- 
Jens Axboe