Re: Linux 2.6.20-rc1

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

On Sat, Dec 16 2006, Linus Torvalds wrote:
> That said: Jens - I think 0e75f906 was a mistake. "blk_rq_unmap()" really 
> should be passed the "struct bio", not the "struct request *". Right now 
> it does something _really_ strange with requests with linked bio's, and I 
> don't think your and FUJITA's "leak fix" really works. What happens when 
> the bio was a linked list on the request, and you put the old _head_ on 
> the request with "rq->bio = bio"? What happens to the other parts of it?

I agree it's fishy and I did think about it. The design isn't exactly
the prettiest, but it should be safe. The reason is that we don't
actually unlink the individual bio from the list, even if we may set
rq->bio to point somewhere further into the list. So as long as the bio
is valid, the bi_next field is still valid as well. We need a reference
on the bio to perform the unmap and blk_rq_unmap_user() drops this
reference on its own, so the bio must be valid.

Taking a rq pointer when we really want a bio is nasty, though. I'll
chance that at least.

> IOW, I think this is broken. I think we should revert 0e75f906. Or at 
> least you should explain to me why it's not broken, and why clearly people 
> (eg Alistair) still see problems with it?

I'm not so sure it's that patch, the same problem seemed to exist for
some people prior to 2.6.20.

-- 
Jens Axboe