On 04/06/2020 05:32 PM, Damien Le Moal wrote:
>> -
>> >- do {
>> >- struct page *page = kmem_to_page(data);
>> >- unsigned int off = offset_in_page(data);
>> >- unsigned int len = min_t(unsigned, left, PAGE_SIZE - off);
>> >-
>> >- while (bio_add_page(bio, page, len, off) != len) {
>> >- struct bio *prev = bio;
>> >-
>> >- bio = bio_alloc(GFP_KERNEL, bio_max_vecs(left));
>> >- bio_copy_dev(bio, prev);
>> >- bio->bi_iter.bi_sector = bio_end_sector(prev);
>> >- bio->bi_opf = prev->bi_opf;
>> >- bio_chain(prev, bio);
>> >-
>> >- submit_bio(prev);
>> >- }
>> >-
>> >- data += len;
>> >- left -= len;
>> >- } while (left > 0);
> Your helper could use a similar loop structure. This is very easy to read.
>
If I understand correctly this pattern is used since it is not a part of
block layer.
The helpers in blk-lib.c are not accessible so this :-
1. Adds an extra helper bio_max_vecs().
2. Adds a new macro howmany which is XFS specific and doesn't
follow usual block layer macros naming.
3. Open codes bio chaining.
4. Uses two bio variables for chaining.
5. Doesn't allow to anchor bio which is needed in async I/O scenario.
All above breaks the existing pattern and code reuse in blk-lib.c, since
blk-lib.c:-
1. Already provides blk_next_bio() why repeat the bio allocation
and bio chaining code ?
2. Instead of adding a new helper bio_max_vecs() why not use existing
__blkdev_sectors_to_bio_pages() ?
3. Why use two bios when it can be done with one bio with the helpers
in blk-lib.c ?
4. Allows async version.
