Re: [PATCH 2/2] block: add a bvec_phys helper

[Christoph Hellwig <hch@lst.de>]

On Fri, Jul 05, 2024 at 10:50:31AM +0200, Geert Uytterhoeven wrote:
> > +               seg_size = get_max_segment_size(lim, bvec_phys(bv) + total_len);
> >                 seg_size = min(seg_size, len);
> >
> >                 (*nsegs)++;
> > @@ -492,8 +491,7 @@ static unsigned blk_bvec_map_sg(struct request_queue *q,
> >         while (nbytes > 0) {
> >                 unsigned offset = bvec->bv_offset + total;
> >                 unsigned len = min(get_max_segment_size(&q->limits,
> > -                                  page_to_phys(bvec->bv_page) + offset),
> > -                                  nbytes);
> > +                                  bvec_phys(bvec) + total), nbytes);
> >                 struct page *page = bvec->bv_page;
> >
> >                 /*
> 
> If you would have introduce bvec_phys() first, you could fold the above
> two hunks into [PATCH 1/2].

Not sure what the advantage of that is, though?

> Which suggests this is arch-specific, and may not always be defined
> the same? I checked a few (but not all) that seem to differ from the
> above at first sight, but end up doing the same...
> 
> I think it would be good to make sure they are identical, and if
> they are, move them to a common place first, to any subtle breakages.

It fundamentally is a wrapper around page_to_pfn that converts from
the PFN to the full physical address.  There a bunch of weird
incosnsitencies and it should really move to common code, but I don't
want this series to depend on that.  The only interesting part is
that for architectures with physical addresses larger than unsigned long
we need to cast to a 64-bit type, although all the architectures that
actually do that in the page_to_phys helper itself do that incorrectly
by casting to a dma_addr_t instdead of a phys_addr_t.  Fortunately we've
stopped supporting a dma_addr_t smaller than phys_addr_t a long time ago,
and even back then that only affected sparc (IIRC).