Tejun,

On Thu, Aug 9, 2012 at 12:01 AM, Tejun Heo [off-list ref] wrote:

Hello,

On Wed, Aug 08, 2012 at 04:47:46PM -0700, Muthu Kumar wrote:

quoted

You are changing the meaning of __bio_clone() here. In old code, the
number of io_vecs, bi_idx, bi_vcnt are preserved. But in this modified
code, you are mapping bio_src's bi_iovec[bi_idx] to bio_dests
bi_iovec[0] and also restricting the number of allocated io_vecs of
the clone. It may be useful for cases were we would like a identical
copy of the original bio (may not be in current code base, but this
implementation is definitely not what one would expect from the name
"clone").

Implementation details changed somewhat but the high-level semantics
didn't change at all.  Any driver not messing with bio internals - and
they shouldn't - shouldn't notice the change.

The reason for doing this change is because the code in question is
messing with bio internals.

No in-kernel drivers

seem to be broken by the change.  If you ask me, this looks more like
a bug fix to me where the bug is a silly behavior restricting
usefulness of the interface.

quoted

May be, call this new implementation some thing else (and use it for bcache)?

This doesn't only change __bio_clone() but all clone interface stacked
on top of it, so, no way.

This ain't windows.

ah... when you put it this way, it gets a different perspective :)

Anyway, my point is, we shouldn't make it non-obvious ("clone" should
be just "clone"). But, we can always add more comments i guess.

Regards,
Muthu

Thanks.

--
tejun