Re: [PATCH 3/6] bio: add allocation cache abstraction

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

On Thu, Aug 12, 2021 at 09:08:29AM -0600, Jens Axboe wrote:
> >> +	cache = per_cpu_ptr(bs->cache, get_cpu());
> >> +	bio = bio_list_pop(&cache->free_list);
> >> +	if (bio) {
> >> +		cache->nr--;
> >> +		put_cpu();
> >> +		bio_init(bio, nr_vecs ? bio->bi_inline_vecs : NULL, nr_vecs);
> >> +		bio_set_flag(bio, BIO_PERCPU_CACHE);
> >> +		return bio;
> >> +	}
> >> +	put_cpu();
> >> +normal_alloc:
> >> +	bio = bio_alloc_bioset(gfp, nr_vecs, bs);
> >> +	if (cache)
> >> +		bio_set_flag(bio, BIO_PERCPU_CACHE);
> >> +	return bio;
> > 
> > The goto here is pretty obsfucating and adds an extra patch to the fast
> > path.
> 
> I don't agree, and it's not the fast path - the fast path is popping off
> a bio off the list, not hitting the allocator.

Oh, I see you special case the list pop return now.  Still seems much
easier to follow to avoid the goto, the cache initialization and the
conditional in the no bio found in the list case (see patch below).

diff --git a/block/bio.c b/block/bio.c
index 689335c00937..b42621cecbef 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1707,11 +1707,11 @@ EXPORT_SYMBOL(bioset_init_from_src);
 struct bio *bio_alloc_kiocb(struct kiocb *kiocb, gfp_t gfp,
 			    unsigned short nr_vecs, struct bio_set *bs)
 {
-	struct bio_alloc_cache *cache = NULL;
+	struct bio_alloc_cache *cache;
 	struct bio *bio;
 
 	if (!(kiocb->ki_flags & IOCB_ALLOC_CACHE) || nr_vecs > BIO_INLINE_VECS)
-		goto normal_alloc;
+		return bio_alloc_bioset(gfp, nr_vecs, bs);
 
 	cache = per_cpu_ptr(bs->cache, get_cpu());
 	bio = bio_list_pop(&cache->free_list);
@@ -1723,10 +1723,8 @@ struct bio *bio_alloc_kiocb(struct kiocb *kiocb, gfp_t gfp,
 		return bio;
 	}
 	put_cpu();
-normal_alloc:
 	bio = bio_alloc_bioset(gfp, nr_vecs, bs);
-	if (cache)
-		bio_set_flag(bio, BIO_PERCPU_CACHE);
+	bio_set_flag(bio, BIO_PERCPU_CACHE);
 	return bio;
 }