Re: [PATCH 2/2] xfs: use block layer helper for rw

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.





[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux