Jens Axboe wrote:
> On Thu, Mar 19 2009, Boaz Harrosh wrote:
>> Jens Axboe wrote:
>>> 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);
>>>> - }
>> This is the code I have today in osd_initiator.c (In scsi-misc tree)
>>
>>> 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!
>>>
>> Then the above code taken from today's osd_initiator is good? People where
>> complaining that it is not good, because it is messing up with internal
>> block structures.
>>
>> What about just exporting an blk_rq_abort(struct request *req) that is
>> the same as PATCH 1/2. But is not called from blk_put_request ?
>
> Well no, the approach isn't that good either. How did you map these
> request? Most logical API would have something to unmap them again.
>
I'm not sure I understand.
I use a combination of map functions to build a complex bio. But more
specifically I do not want to use any unmap function because I do not
want to call the end_bio() function I want to not allow any bouncing
unmapping or hooking to occur. "Abort" is the only write name for it.
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
