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

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 <[email protected]>
> ---
>  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 <[email protected]>


-- 
Damien Le Moal
Western Digital Research