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:
> > #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.
>
> > +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)
> > +{
> > + 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.
