Re: [PATCH v3 2/3] btrfs: remove btrfs_bio_alloc() helper

[David Sterba <dsterba@suse.cz>]

On Fri, Sep 17, 2021 at 03:27:44PM +0300, Nikolay Borisov wrote:
> > -struct bio *btrfs_bio_alloc(u64 first_byte)
> > +struct bio *btrfs_io_bio_alloc(unsigned int nr_iovecs)
> >  {
> >  	struct bio *bio;
> >  
> > -	bio = bio_alloc_bioset(GFP_NOFS, BIO_MAX_VECS, &btrfs_bioset);
> > -	bio->bi_iter.bi_sector = first_byte >> 9;
> > +	ASSERT(nr_iovecs <= BIO_MAX_VECS);
> > +	if (nr_iovecs == 0)
> > +		nr_iovecs = BIO_MAX_VECS;
> 
> hell no! How come passing 0 actually means BIO_MAX_VEC. Instead of
> having 0 everywhere and have the function translate this to
> BIO_MAX_VECS, simply pass BIO_MAX_VECS in every call site where it's
> needed.

I had thought about that before and cocluded that passing BIO_MAX_VECS
everywhere would be the wrong way as it's a detail about how many bio
vecs are allocated. So 0 is a default is ok as any other number means
that it's the exact count.

> David, please either fix the patch in the tree or retract it. Let's try
> and refrain from adding such "gems" to the code base.

So should we add another helper that takes the exact number and drop the
parameter everwhere is 0 so it's just btrfs_io_bio_alloc() with the
fallback?