Re: [PATCH] block: Revert bio_clone() default behaviour

[Chris Mason <chris.mason@fusionio.com>]

Quoting Kent Overstreet (2013-11-06 15:57:34)
> On Wed, Nov 06, 2013 at 03:22:36PM -0500, Chris Mason wrote:
> > Quoting Kent Overstreet (2013-11-06 15:02:22)

[ ... nods, thanks! ... ]

> OTOH - with regards to just the ordering requirements, the more I look at
> various code the less accidental the fact that that works seems to be: the best
> explanation I've come up with so far is that you already needed to ensure that
> the _pages_ the clone points to stick around until the clone completes, and if
> you don't own the original bio the only way to do that is to not complete the
> original bio until after the clone completes.
> 
> So if you're a driver cloning bios that were submitted to you, bio_clone_fast()
> introduces no new ordering requirements.
> 
> On the third hand - if you're cloning (i.e. splitting) your own bios, in that
> case it would be possible to screw up the ordering - I don't know of any code in
> the kernel that does this today (except for, sort of, bcache) but my dio rewrite
> takes this approach - but if you do the obvious and sane thing with bio_chain()
> it's a non issue, it seems to me you'd have to come up with something pretty
> contrived and dumb for this to actually be an issue in practice.
> 
> Anyways, I haven't come to any definite conclusions, those are just my
> observations so far.

I do think you're right.  We all seem to have clones doing work on
behalf of the original, and when everyone is done we complete the
original.

But, btrfs does have silly things like this:

        dio_end_io(dio_bio, err); // end and free the original
	bio_put(bio); // free the clone

It's not a bug yet, but given enough time the space between those two
frees will grow new code that kills us all.

Really though, the new stuff is better, thanks.

-chris