Re: [PATCH 2/2] block: share more code for bio addition helpers

[Kundan Kumar <kundan.kumar@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]

On 05/11/24 04:52PM, Christoph Hellwig wrote:
>__bio_iov_iter_get_pages currently open codes adding pages to the bio,
>which duplicates a lot of code from bio_add_page.  Add a lower level
>bio_do_add_page helpers that passes down the same_page output argument
>so that __bio_iov_iter_get_pages can reuse the code and also check
>for the error return from it - while no error should happen due to
>how the bvec space is used for the pin_user_space result I'd rather
>be safe than sorry and actually check for these impossible errors.
>
>Signed-off-by: Christoph Hellwig <hch@lst.de>
>---
> block/bio.c | 66 ++++++++++++++++++++++++-----------------------------
> 1 file changed, 30 insertions(+), 36 deletions(-)
>
>diff --git a/block/bio.c b/block/bio.c
>index 1f6ac44b4881..bc3bca5f0686 100644
>--- a/block/bio.c
>+++ b/block/bio.c
>@@ -1063,20 +1063,9 @@ void __bio_add_page(struct bio *bio, struct page *page,
> }
> EXPORT_SYMBOL_GPL(__bio_add_page);
>
>-/**
>- *	bio_add_page	-	attempt to add page(s) to bio
>- *	@bio: destination bio
>- *	@page: start page to add
>- *	@len: vec entry length, may cross pages
>- *	@offset: vec entry offset relative to @page, may cross pages
>- *
>- *	Attempt to add page(s) to the bio_vec maplist. This will only fail
>- *	if either bio->bi_vcnt == bio->bi_max_vecs or it's a cloned bio.
>- */
>-int bio_add_page(struct bio *bio, struct page *page,
>-		 unsigned int len, unsigned int offset)
>+static int bio_do_add_page(struct bio *bio, struct page *page,
>+		 unsigned int len, unsigned int offset, bool *same_page)

As we are passing length within a folio, values will reach near UINT_MAX.
It will be better to make len and offset as size_t, also to add a check like :
if (len > UINT_MAX || offset > UINT_MAX)
		return 0; 

> {
>-	bool same_page = false;

nit: extra line got added


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]