Re: [RFC] cleanup bcache bio handling

Kent Overstreet
2018-06-13T14:54:09Z

On Wed, Jun 13, 2018 at 03:56:32PM +0200, Christoph Hellwig wrote:
> On Wed, Jun 13, 2018 at 07:06:41PM +0800, Ming Lei wrote:
> > > before bio_alloc_pages) that can be switched to something that just creates a
> > > single bvec.
> >
> > Yes, multipage bvec shouldn't break any driver or fs.
>
> It probably isn't broken, at least I didn't see assumptions of the same
> number of segments. However the current poking into the bio internals as
> a bad idea for a couple of reasons. First because it requires touching
> bcache for any of these changes, second because it won't get merging of
> pages into a single bio segment for bіos built by bch_bio_map or
> bch_bio_alloc_pages, and third bcache is the last user of
> bio_for_each_chunk_all in your branch, which I'd like to kill off to
> keep the number of iterators down.

Agreed about bio_for_each_chunk_all(), but I just looked at the patch that
introduces them and it looks to me like there's no need, they should just be
bio_for_each_segment_all().

Converting bch_bio_map() and bch_bio_alloc_pages() to bio_add_page() is fine by
me, but your patch series doesn't do any of those actual cleanups: your
description of the patch series does not actually match what it does.