Re: [PATCH 11/27] bcache: io.c: use bio_set_vec_table

[Ming Lei]

On Tue, Apr 5, 2016 at 8:49 PM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> On Tue, Apr 05, 2016 at 07:56:56PM +0800, Ming Lei wrote:
>> diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
>> index 86a0bb8..1c48462 100644
>> --- a/drivers/md/bcache/io.c
>> +++ b/drivers/md/bcache/io.c
>> @@ -26,8 +26,7 @@ struct bio *bch_bbio_alloc(struct cache_set *c)
>>
>>       bio_init(bio);
>>       bio->bi_flags           |= BIO_POOL_NONE << BIO_POOL_OFFSET;
>> -     bio->bi_max_vecs         = bucket_pages(c);
>> -     bio->bi_io_vec           = bio->bi_inline_vecs;
>> +     bio_set_vec_table(bio, bio->bi_inline_vecs, bucket_pages(c));
>
> All this bcache code needs to move away from bio_init on a bio
> embedded in a driver private structure toward properly using
> bio_alloc / bio_alloc_bioset.  That will also fix the crash
> with bcache over md that Shaohua reported, so I'd suggest to fast
> track this part of the series.

I suggest to keep this usage for the following reasons:

- bio can be embedded into one biger instance, which is often allocated
dynamically, so one extra allocation for bio can be avoided.

- we should support arbitrary bio size by this way, at least bio_add_page()
supports this usage.  Also code gets lots of simplication with arbitrary bio
size support, such as prio_io(): bcache

BTW, the root cause for bcache crash still isn't clear now because
blk_bio_segment_split() should split big bio into proper size with
all queue's limits. Maybe the max segment limit isn't figured out correctly.

Thanks,
Ming Lei