Message-ID: <20131106204023.GB3842@kmo>
Date:	Wed, 6 Nov 2013 12:40:23 -0800
From:	Kent Overstreet <kmo@...erainc.com>
To:	Mike Snitzer <snitzer@...hat.com>
Cc:	axboe@...nel.dk, linux-kernel@...r.kernel.org,
	Chris Mason <chris.mason@...ionio.com>,
	NeilBrown <neilb@...e.de>, Olof Johansson <olof@...om.net>
Subject: Re: block: Revert bio_clone() default behaviour

On Wed, Nov 06, 2013 at 03:31:02PM -0500, Mike Snitzer wrote:
> Hey Kent,
> 
> Digging a bit in the LKML archive I think this patch is in response to
> this thread: https://lkml.org/lkml/2013/11/6/27

That thread I saw, Jens told me there was another one though

> Might be good to give context for which reported problem(s) are being
> fixed by this patch.
> 
> On Tue, Nov 05 2013 at 10:48pm -0500,
> Kent Overstreet <kmo@...erainc.com> wrote:
> 
> > This patch reverts the default behaviour introduced by
> > 9fc6286f347d00528adcdcf12396d220f47492ed - bio_clone_biovec() no clonger
> > shares the source bio's biovec, cloning the biovec is once again the
> > default.
> 
> Your focus, in terms of revert, seems to be on restoring
> bio_clone_bioset, so: s/bio_clone_biovec/bio_clone_bioset/
> 
> Maybe best to say "effectively reverts" since you aren't reverting 
> 9fc6286f347d00528adcdcf12396d220f47492ed ?  Also s/clonger/longer/ typo ^
> 
> > Instead, we add a new bio_clone_biovec_fast(), which creates a clone
> > that shares the source's biovec. This patch changes bcache and md to use
> 
> s/md/dm/

Whoops :p

> > __bio_clone_biovec_fast() since they're expecting the new behaviour due
> > to other refactoring; most of the other uses of bio_clone() should be
> > same to convert to the _fast() variant but that will be done more
> 
> s/same/safe/

Thanks

> 
> > incrementally in other patches (bio_split() in particular).
> > 
> > Note that __bio_clone() isn't being readded - the reason being that with
> > immutable biovecs allocating the right number of biovecs for the new
> > clone is no longer trivial so we don't want drivers trying to do that
> > themselves.
> > 
> > This patch also reverts febca1baea1cfe2d7a0271385d89b03d5fb34f94 -
> > __bio_clone_fast() should not be setting bi_vcnt for bios that do not
> > own the biovec (see Documentation/block/biovecs.txt for rationale) - in
> > short, not setting it might cause bugs in the short term but long term
> > it's likely to hide nastier more subtle bugs, we don't want code looking
> > at bi_vcnt at all for bios it does not own. However, this patch
> > _shouldn't_ cause any regressions because of this since we're reverting
> > back to the old bio_clone() behaviour.
> > 
> > Signed-off-by: Kent Overstreet <kmo@...erainc.com>
> > Cc: Jens Axboe <axboe@...nel.dk>
> > Cc: Chris Mason <chris.mason@...ionio.com>
> > Cc: Mike Snitzer <snitzer@...hat.com>
> > Cc: NeilBrown <neilb@...e.de>
> > Cc: Olof Johansson <olof@...om.net>
> > ---
> > Chris, Olaf, can you two in particular test this? I have tested the bounce
> > buffer code (and bcache), but Jens told me today there was an md bug that I
> > _still_ can't find any emails about so I'm not sure what to test for that.
> 
> /me assumes you really mean md here, given Chris's later reply in this thread.
> 
> Relative to DM, this patch looks fine to me:
> 
> Acked-by: Mike Snitzer <snitzer@...hat.com>

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists