Re: [PATCH V2] block: optimize for small BS IO

[Ming Lei <ming.lei@redhat.com>]

On Tue, Oct 29, 2019 at 12:27:45AM -0700, Christoph Hellwig wrote:
> On Tue, Oct 29, 2019 at 03:06:21PM +0800, Ming Lei wrote:
> > __blk_queue_split() may be a bit heavy for small BS(such as 512B, or
> 
> Maybe spell out block size.  BS has another much less nice connotation.

OK.

> 
> > bch_bio_map() should be the only one which doesn't use bio_add_page(),
> > so force to mark bio built via bch_bio_map() as MULTI_PAGE.
> 
> We really need to fix that up.  I had patches back in the day which
> Kent didn't particularly like for non-technical reason, that might serve
> as a starting point.
> 
> > @@ -789,6 +794,10 @@ void __bio_add_page(struct bio *bio, struct page *page,
> >  	bio->bi_iter.bi_size += len;
> >  	bio->bi_vcnt++;
> >  
> > +	if (!bio_flagged(bio, BIO_MULTI_PAGE) && (bio->bi_vcnt >= 2 ||
> > +				(bio->bi_vcnt == 1 && len > PAGE_SIZE)))
> > +		bio_set_flag(bio, BIO_MULTI_PAGE);
> 
> This looks pretty ugly and does more (and more confusing) checks than
> actually needed Maybe we need a little bio_is_multi_page helper to clean
> this up a bit:
> 
> /*
>  * Check if the bio contains more than a page and thus needs special
>  * treatment in the bio splitting code.
>  */
> static inline bool bio_is_multi_page(struct bio *bio)
> {
> 	return bio->bi_vcnt > 1 || bio->bi_io_vec[0].bv_len > PAGE_SIZE;
> }
> 
> and then this becomes:
> 
> 	if (!bio_flagged(bio, BIO_MULTI_PAGE) && bio_is_multi_page(bio))
> 
> Then again these checks are so cheap that we can just use the
> bio_is_multi_page helper directly and skip the flag entirely.

I'd suggest to not add this helper:

1) there is only one user

2) the helper has to refer to bio->bi_io_vec

However, the above check can be simplified as:

	if (!bio_flagged(bio, BIO_MULTI_PAGE) && (bio->bi_vcnt >= 2 ||
				bv->bv_len > PAGE_SIZE))
		bio_set_flag(bio, BIO_MULTI_PAGE);

Then the check has basically zero cost since all the checked variables
are just written or read in __bio_add_page() before the check.

Thanks,
Ming