Re: [PATCH 00/17] md: cleanup on direct access to bvec table

[Shaohua Li]

On Fri, Feb 17, 2017 at 09:25:27AM +0800, Ming Lei wrote:
> Hi Shaohua,
> 
> On Fri, Feb 17, 2017 at 6:16 AM, Shaohua Li  wrote:
> > On Thu, Feb 16, 2017 at 07:45:30PM +0800, Ming Lei wrote:
> >> In MD's resync I/O path, there are lots of direct access to bio's
> >> bvec table. This patchset kills most of them, and the conversion
> >> is quite straightforward.
> >
> > I don't like this approach. The MD uses a hacky way to manage pages 
> > allocated,
> > this is the root of the problem. The patches add another hack way to do the
> 
> Yes, I agree, and bio_iov_iter_get_pages() uses this kind of hacky way too
> actually.
> 
> > management. I'd like to see explict management of the pages, for example, 
> > add
> > data structure in r1bio to manage the pages, then we can use existing API 
> > for
> > all the stuffes we need.
> 
> Yeah, that is definitely clean, but we have to pay the following cost:
> 
> - allocate at least N * (128 + 4) bytes per each r1_bio/r10_bio
> - N is pool_info.raid_disks for raid1, and conf->copies for raid10
> 
> If we are happy to introduce the cost, I can take this way in V1.

It's not a big deal. The inflight bio shouldn't be big, so the r1_bio count
isn't big. We don't waste much.

Thanks,
Shaohua

Re: [PATCH 00/17] md: cleanup on direct access to bvec table

[Ming Lei]

Hi Shaohua,

On Fri, Feb 17, 2017 at 6:16 AM, Shaohua Li  wrote:
> On Thu, Feb 16, 2017 at 07:45:30PM +0800, Ming Lei wrote:
>> In MD's resync I/O path, there are lots of direct access to bio's
>> bvec table. This patchset kills most of them, and the conversion
>> is quite straightforward.
>
> I don't like this approach. The MD uses a hacky way to manage pages allocated,
> this is the root of the problem. The patches add another hack way to do the

Yes, I agree, and bio_iov_iter_get_pages() uses this kind of hacky way too
actually.

> management. I'd like to see explict management of the pages, for example, add
> data structure in r1bio to manage the pages, then we can use existing API for
> all the stuffes we need.

Yeah, that is definitely clean, but we have to pay the following cost:

- allocate at least N * (128 + 4) bytes per each r1_bio/r10_bio
- N is pool_info.raid_disks for raid1, and conf->copies for raid10

If we are happy to introduce the cost, I can take this way in V1.


Thanks,
Ming Lei

[PATCH 00/17] md: cleanup on direct access to bvec table

[Ming Lei]

In MD's resync I/O path, there are lots of direct access to bio's
bvec table. This patchset kills most of them, and the conversion
is quite straightforward.

Once direct access to bvec table in MD is cleaned up, we may make
multipage bvec moving on.

Thanks,
Ming

Ming Lei (17):
  block: introduce bio_segments_all()
  block: introduce bio_remove_last_page()
  md: raid1/raid10: use bio_remove_last_page()
  md: introduce helpers for dealing with fetch/store preallocated pages
in bio
  md: raid1/raid10: use the introduced helpers
  md: raid1/raid10: borrow .bi_error as pre-allocated page index
  md: raid1/raid10: don't use .bi_vcnt to check if all pages are added
  md: raid1: simplify r1buf_pool_free()
  md: raid1/raid10: use bio helper in *_pool_free
  md: raid1: remove direct access to bvec table in fix_sync_read_error
  md: raid1: use bio helper in process_checks()
  md: raid1: avoid direct access to bvec table in process_checks()
  md: raid1: use bio_segments_all()
  md: raid10: avoid direct access to bvec table in sync_request_write()
  md: raid10: avoid direct access to bvec table in
fix_recovery_read_error
  md: raid10: avoid direct access to bvec table in reshape_request
  md: raid10: avoid direct access to bvec table in
handle_reshape_read_error

 block/bio.c | 23 ++
 drivers/md/md.h | 21 +
 drivers/md/raid1.c  | 87 +
 drivers/md/raid10.c | 76 ++
 include/linux/bio.h |  8 +
 5 files changed, 169 insertions(+), 46 deletions(-)

-- 
2.7.4