Re: [PATCH v2 09/13] md: raid1: use bio_segments_all()

On Fri, Mar 3, 2017 at 10:20 AM, Ming Lei [off-list ref] wrote:

On Thu, Mar 2, 2017 at 3:52 PM, Shaohua Li [off-list ref] wrote:

quoted

On Thu, Mar 02, 2017 at 10:34:25AM +0800, Ming Lei wrote:

quoted

Hi Shaohua,

On Wed, Mar 1, 2017 at 7:42 AM, Shaohua Li [off-list ref] wrote:

quoted

On Tue, Feb 28, 2017 at 11:41:39PM +0800, Ming Lei wrote:

quoted

Use this helper, instead of direct access to .bi_vcnt.

what We really need to do for the behind IO is:
- allocate memory and copy bio data to the memory
- let behind bio do IO against the memory

The behind bio doesn't need to have the exactly same bio_vec setting. If we
just track the new memory, we don't need use the bio_segments_all and access
bio_vec too.

But we need to figure out how many vecs(each vec store one page) to be
allocated for the cloned/behind bio, and that is the only value of
bio_segments_all() here. Or you have idea to avoid that?

As I said, the behind bio doesn't need to have the exactly same bio_vec
setting. We just allocate memory and copy original bio data to the memory,
then do IO against the new memory. The behind bio
segments == (bio->bi_iter.bi_size + PAGE_SIZE - 1) >> PAGE_SHIFT

The equation isn't always correct, especially when bvec includes just
part of page, and it is quite often in case of mkfs, in which one bvec often
includes 512byte buffer.

Think it further, your idea could be workable and more clean, but the change
can be a bit big, looks we need to switch handling write behind into
the following way:

1) replace bio_clone_bioset_partial() with bio_allocate(nr_vecs), and 'nr_vecs'
is computed with your equation;

2) allocate 'nr_vecs' pages once and share them among all created bio in 1)

3) for each created bio, add each page into the bio via bio_add_page()

4) only for the 1st created bio, call bio_copy_data() to copy data from
master bio.

Let me know if you are OK with the above implementaion.


Thanks,
Ming Lei