diff mbox series

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

Message ID 20230731125459epcms2p177a5cc5caa7ef0a9de35689e96558f43@epcms2p1
State New
Headers
Series multi-page bvec configuration for integrity payload |

Commit Message

Jinyoung Choi July 31, 2023, 12:54 p.m. UTC 
  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(-)

Comments

Christoph Hellwig Aug. 1, 2023, 10:03 a.m. UTC | #1 
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>
Jinyoung Choi Aug. 3, 2023, 1:48 a.m. UTC | #2 
> 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.
diff mbox series

Patch

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);