Re: [PATCH 03/17] block: add a bio_add_vmalloc helper

Damien Le Moal
2025-04-24T06:06:50Z

On 4/22/25 23:26, Christoph Hellwig wrote:
> Add a helper to add a vmalloc region to a bio, abstracting away the
> vmalloc addresses from the underlying pages. Also add a helper to
> calculate how many segments need to be allocated for a vmalloc region.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> block/bio.c | 27 +++++++++++++++++++++++++++
> include/linux/bio.h | 17 +++++++++++++++++
> 2 files changed, 44 insertions(+)
>
> diff --git a/block/bio.c b/block/bio.c
> index a6a867a432cf..3cc93bbdeeb9 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1058,6 +1058,33 @@ bool bio_add_folio(struct bio *bio, struct folio *folio, size_t len,
> }
> EXPORT_SYMBOL(bio_add_folio);
>
> +/**
> + * bio_add_vmalloc - add a vmalloc region to a bio
> + * @bio: destination bio
> + * @vaddr: virtual address to add
> + * @len: total length of the data to add

Nit: to be consistent in the wording...

* @vaddr: address of the vmalloc region to add
* @len: total length of the vmalloc region to add

> + *
> + * Add the data at @vaddr to @bio and return how much was added. This can an

s/an/and

or may be simply:

This may be less than the amount originally asked.

> + * usually is less than the amount originally asked. Returns 0 if no data could
> + * be added to the bio.
> + *
> + * This helper calls flush_kernel_vmap_range() for the range added. For reads,
> + * the caller still needs to manually call invalidate_kernel_vmap_range() in
> + * the completion handler.
> + */
> +unsigned int bio_add_vmalloc(struct bio *bio, void *vaddr, unsigned len)
> +{
> + unsigned int offset = offset_in_page(vaddr);
> +
> + len = min(len, PAGE_SIZE - offset);
> + if (bio_add_page(bio, vmalloc_to_page(vaddr), len, offset) < len)
> + return 0;
> + if (op_is_write(bio_op(bio)))
> + flush_kernel_vmap_range(vaddr, len);
> + return len;
> +}
> +EXPORT_SYMBOL_GPL(bio_add_vmalloc);
> +
> void __bio_release_pages(struct bio *bio, bool mark_dirty)
> {
> struct folio_iter fi;
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 17a10220c57d..c4069422fd0a 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -433,6 +433,23 @@ static inline void bio_add_virt_nofail(struct bio *bio, void *vaddr,
> __bio_add_page(bio, virt_to_page(vaddr), len, offset_in_page(vaddr));
> }
>
> +/**
> + * bio_vmalloc_max_vecs - number of segments needed to map vmalloc data

Nit: number of BIO segments needed to add a vmalloc-ed region to a BIO ?

> + * @vaddr: address to map
> + * @len: length to map

Nit:

* @vaddr: address of the vmalloc region to add
* @len: total length of the vmalloc region to add

> + *
> + * Calculate how many bio segments need to be allocated to map the vmalloc/vmap

s/to map/to add ?

> + * range in [@addr:@len]. This could be an overestimation if the vmalloc area
> + * is backed by large folios.
> + */
> +static inline unsigned int bio_vmalloc_max_vecs(void *vaddr, unsigned int len)
> +{
> + return DIV_ROUND_UP(offset_in_page(vaddr) + len, PAGE_SIZE);
> +}
> +
> +unsigned int __must_check bio_add_vmalloc(struct bio *bio, void *vaddr,
> + unsigned len);
> +
> int submit_bio_wait(struct bio *bio);
> int bdev_rw_virt(struct block_device *bdev, sector_t sector, void *data,
> size_t len, enum req_op op);

Other than these wording nits, looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

--
Damien Le Moal
Western Digital Research