On Thu, Jun 28, 2018 at 04:54:44PM -0700, Bart Van Assche wrote:

On 06/28/18 16:16, Kent Overstreet wrote:

quoted

On Fri, Jun 29, 2018 at 07:10:47AM +0800, Ming Lei wrote:

quoted

On Thu, Jun 28, 2018 at 11:21 PM, Bart Van Assche
[off-list ref] wrote:

quoted

On 06/27/18 17:30, Ming Lei wrote:

quoted

One core idea of immutable bvec is to use bio->bi_iter and the original
bvec table to iterate over anywhere in the bio. That is why .bi_io_vec
needs to copy, but not see any reason why .bi_vcnt needs to do.

Do you have use cases on .bi_vcnt for cloned bio?

So far this is only a theoretical concern. There are many functions in the
block layer that use .bi_vcnt, and it is a lot of work to figure out all the
callers of all these functions.

Back when I implemented immutable biovecs I thoroughly audited all the bi_vcnt
uses and removed all of them that weren't by the code that owns/submits the bio.

Grepping around I see one or two suspicious uses.. blk-merge.c in particular

quoted

No, any functions using .bi_vcnt on a cloned-bio may be a bug, and we should
take a close look.

not just cloned bios, any code using bi_vcnt on a bio it didn't create is wrong.

so big nack to this patch (I wasn't ccd on it though and it doesn't seem to have
hit lkml, so I can't find the original patch...)

Hello Kent,

Thanks for chiming in. The linux-block mailing list is archived by multiple
websites. The entire e-mail thread is available on e.g.
https://www.mail-archive.com/linux-block@vger.kernel.org/msg23006.html.

I have a question for you: at least in kernel v4.17 bio_clone_bioset()
copies bi_vcnt from the source to the destination bio. However,
__bio_clone_fast() doesn't copy bi_vcnt. Isn't that an inconsistency?

No - when you use bio_clone_bioset() you get a bio that you own and can do
whatever you want with, so it does make sense for it to initialize bi_vcnt.

e.g. you could use bio_clone_bioset() when you're going to be bouncing a bio,
iterating over each bvec and allocating a new page and copying data from the old
page to the new page.