[PATCH v2 3/4] bio-integrity: cleanup adding integrity pages to bip's bvec

Jinyoung Choi posted 4 patches 2 years, 6 months ago
There is a newer version of this series
[PATCH v2 3/4] bio-integrity: cleanup adding integrity pages to bip's bvec
Posted by Jinyoung Choi 2 years, 6 months ago
The bio_integrity_add_page() returns the set length if the execution
result is successful. Otherwise, return 0.

Unnecessary if statement was removed. And when the result value was less
than the set value, it was changed to failed.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/bio-integrity.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 6220a99977a4..c6b3bc86e1f9 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -252,27 +252,18 @@ bool bio_integrity_prep(struct bio *bio)
 
 	/* Map it */
 	offset = offset_in_page(buf);
-	for (i = 0 ; i < nr_pages ; i++) {
-		int ret;
+	for (i = 0; i < nr_pages && len > 0; i++) {
 		bytes = PAGE_SIZE - offset;
 
-		if (len <= 0)
-			break;
-
 		if (bytes > len)
 			bytes = len;
 
-		ret = bio_integrity_add_page(bio, virt_to_page(buf),
-					     bytes, offset);
-
-		if (ret == 0) {
+		if (bio_integrity_add_page(bio, virt_to_page(buf),
+					   bytes, offset) < bytes) {
 			printk(KERN_ERR "could not attach integrity payload\n");
 			goto err_end_io;
 		}
 
-		if (ret < bytes)
-			break;
-
 		buf += bytes;
 		len -= bytes;
 		offset = 0;
@@ -291,7 +282,6 @@ bool bio_integrity_prep(struct bio *bio)
 	bio->bi_status = BLK_STS_RESOURCE;
 	bio_endio(bio);
 	return false;
-
 }
 EXPORT_SYMBOL(bio_integrity_prep);
 
-- 
2.34.1
Re: [PATCH v2 3/4] bio-integrity: cleanup adding integrity pages to bip's bvec
Posted by Christoph Hellwig 2 years, 6 months ago
On Mon, Jul 31, 2023 at 09:54:59PM +0900, Jinyoung Choi wrote:
> The bio_integrity_add_page() returns the set length if the execution
> result is successful. Otherwise, return 0.
> 
> Unnecessary if statement was removed. And when the result value was less
> than the set value, it was changed to failed.

Maybe word this as

bio_integrity_add_page() returns the add length if successful, else 0,
just as bio_add_page.  Simply check return value checking in
bio_integrity_prep to not deal with a > 0 but < len case that can't
happen.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
RE:(2) [PATCH v2 3/4] bio-integrity: cleanup adding integrity pages to bip's bvec
Posted by Jinyoung Choi 2 years, 6 months ago
> On Mon, Jul 31, 2023 at 09:54:59PM +0900, Jinyoung Choi wrote:
> > The bio_integrity_add_page() returns the set length if the execution
> > result is successful. Otherwise, return 0.
> > 
> > Unnecessary if statement was removed. And when the result value was less
> > than the set value, it was changed to failed.
> 
> Maybe word this as
> 
> bio_integrity_add_page() returns the add length if successful, else 0,
> just as bio_add_page.  Simply check return value checking in
> bio_integrity_prep to not deal with a > 0 but < len case that can't
> happen.
> 
> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Hi, Christoph.
Thank you for your review. I will update comment soon!

Best Regards,
Jinyoung.