On Tue, Jul 20, 2021 at 08:48:07AM +0200, Christoph Hellwig wrote:

On Mon, Jul 19, 2021 at 07:39:46PM +0100, Matthew Wilcox (Oracle) wrote:

quoted

 #define bio_for_each_bvec_all(bvl, bio, i)		\
 	for (i = 0, bvl = bio_first_bvec_all(bio);	\
-	     i < (bio)->bi_vcnt; i++, bvl++)		\
+	     i < (bio)->bi_vcnt; i++, bvl++)

Pleae split out this unrelated fixup.

quoted

+static inline
+void bio_first_folio(struct folio_iter *fi, struct bio *bio, int i)

Please fix the strange formatting.

static inline void bio_first_folio(struct folio_iter *fi, struct bio *bio,
		int i)

quoted

+{
+	struct bio_vec *bvec = bio_first_bvec_all(bio) + i;
+
+	fi->folio = page_folio(bvec->bv_page);
+	fi->offset = bvec->bv_offset +
+			PAGE_SIZE * (bvec->bv_page - &fi->folio->page);

Can we have a little helper for the offset in folio calculation, like:

static inline size_t offset_of_page_in_folio(struct page *page)
{
	return (bvec->bv_page - &page_folio(page)->page) * PAGE;
}

as that makes the callers a lot easier to read.

I've spent most of today thinking about this one.  I actually don't
want to make this easy to read.  This is code that, in an ideal world,
would not exist.  A bio_vec should not contain a struct page; it should
probably be:

struct bio_vec {
	phys_addr_t bv_start;
	unsigned int bv_len;
};

and then the helper to get from a bio_vec to a folio_iter looks like:

	fi->folio = pfn_folio(bvec->bv_start >> PAGE_SHIFT);
	fi->offset = offset_in_folio(fi->folio, bvec->bv_start);

If instead we decide to keep bvecs the way they are, we can at
least turn the bv_page into bv_folio, and then we won't need this
code either.