Hi Christoph,
On Fri, Jul 5, 2024 at 10:15 AM Christoph Hellwig <hch@xxxxxx> wrote:
> Get callers out of poking into bvec internals a bit more. Not a huge win
> right now, but with the proposed new DMA mapping API we might end up with
> a lot more of this otherwise.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Thanks for your patch!
> arch/m68k/emu/nfblock.c | 2 +-
Acked-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -255,8 +255,7 @@ static bool bvec_split_segs(const struct queue_limits *lim,
> unsigned seg_size = 0;
>
> while (len && *nsegs < max_segs) {
> - seg_size = get_max_segment_size(lim, page_to_phys(bv->bv_page) +
> - bv->bv_offset + total_len);
> + 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].
> --- a/include/linux/bvec.h
> +++ b/include/linux/bvec.h
> @@ -280,4 +280,19 @@ static inline void *bvec_virt(struct bio_vec *bvec)
> return page_address(bvec->bv_page) + bvec->bv_offset;
> }
>
> +/**
> + * bvec_phys - return the physical address for a bvec
> + * @bvec: bvec to return the physical address for
> + */
> +static inline phys_addr_t bvec_phys(const struct bio_vec *bvec)
> +{
> + /*
> + * Note this open codes page_to_phys because page_to_phys is defined in
> + * <asm/io.h>, which we don't want to pull in here. If it ever moves to
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.
> + * a sensible place we should start using it.
> + */
> + return ((phys_addr_t)page_to_pfn(bvec->bv_page) << PAGE_SHIFT) +
> + bvec->bv_offset;
> +}
> +
> #endif /* __LINUX_BVEC_H */
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
