[PATCH 00/15] Change the integrity configuration method in block

[PATCH 00/15] Change the integrity configuration method in block

[Jinyoung CHOI]

In the case of NVMe, it has an integrity payload consisting of one segment.
So, rather than configuring SG_LIST, it was changed by direct DMA mapping.

The page-merge is not performed for the struct bio_vec when creating 
a integrity payload in block.
As a result, when creating an integrity paylaod beyond one page, each 
struct bio_vec is generated, and its bv_len does not exceed the PAGESIZE.

To solve it, bio_integrity_add_page() should just add to the existing 
bvec, similar to bio_add_page() and friends. 
As the bip configuration changed, the code related to sg_list was
also modified.

(ref: https://lore.kernel.org/linux-nvme/yq18rewbmay.fsf@ca-mkp.ca.oracle.com/T/#t)


Tested like this:

- Format (support pi)
$ sudo nvme format /dev/nvme2n1 --force -n 1 -i 1 -p 0 -m 0 -l 1 -r

- Run FIO
[global]
ioengine=libaio
group_reporting

[job]
bs=512k
iodepth=256
rw=write
numjobs=8
direct=1
runtime=10s
filename=/dev/nvme2n1

- Result
...
[   93.496218] nvme2n1: I/O Cmd(0x1) @ LBA 62464, 1024 blocks, I/O Error (sct 0x2 / sc 0x82) MORE
[   93.496227] protection error, dev nvme2n1, sector 62464 op 0x1:(WRITE) flags 0x18800 phys_seg 3 prio class 2
[   93.538788] nvme2n1: I/O Cmd(0x1) @ LBA 6144, 1024 blocks, I/O Error (sct 0x2 / sc 0x82) MORE
[   93.538798] protection error, dev nvme2n1, sector 6144 op 0x1:(WRITE) flags 0x18800 phys_seg 3 prio class 2
[   93.566231] nvme2n1: I/O Cmd(0x1) @ LBA 124928, 1024 blocks, I/O Error (sct 0x0 / sc 0x4)
[   93.566241] I/O error, dev nvme2n1, sector 124928 op 0x1:(WRITE) flags 0x18800 phys_seg 3 prio class 2
[   93.694147] nvme2n1: I/O Cmd(0x1) @ LBA 64512, 1024 blocks, I/O Error (sct 0x2 / sc 0x82) MORE
[   93.694155] protection error, dev nvme2n1, sector 64512 op 0x1:(WRITE) flags 0x18800 phys_seg 3 prio class 2
[   93.694299] nvme2n1: I/O Cmd(0x1) @ LBA 5120, 1024 blocks, I/O Error (sct 0x2 / sc 0x82) MORE
[   93.694305] protection error, dev nvme2n1, sector 5120 op 0x1:(WRITE) flags 0x18800 phys_seg 3 prio class 2
...


Jinyoung Choi (15):
  block: bio: rename page_is_mergeable to bio_page_is_mergeable and make
    non-static
  block: blk-integiry: add helper functions for bio_integrity_add_page
  block: bio-integrity: modify bio_integrity_add_page()
  block: bio-integiry: cleanup bio_integrity_prep
  block: fix not to apply bip information in blk_rq_bio_prep()
  block: blk-merge: fix to add the number of integrity segments to the
    request twice
  block: blk-merge: fix merging two requests in ll_merge_requests_fn
  block: add helper function to get the number of integrity segments
  scsi: add scsi_alloc_integrity_sgtables() for integrity process
  scsi: change to use blk_rq_nr_integrity_segments() instead of
    blk_rq_count_integrity_sg()
  block: blk-integrity: change how to find the number of integrity of
    bio
  nvme: rdma: change how to find the number of integrity of request
  block: add helper function for iteration of bip's bvec
  block: blk-integrity: change sg-table configuration method for
    integrity
  block: blk-integrity: remove blk_rq_count_integrity_sg()

 block/bio-integrity.c         | 106 +++++++++++++++++++++------
 block/bio.c                   |   8 +--
 block/blk-integrity.c         | 132 +++++++++-------------------------
 block/blk-merge.c             |  66 +++++++++++++++--
 block/blk.h                   |   7 ++
 drivers/nvme/host/rdma.c      |   2 +-
 drivers/scsi/scsi_lib.c       |  67 ++++++++---------
 include/linux/bio.h           |   7 ++
 include/linux/blk-integrity.h |  10 ++-
 include/linux/blk-mq.h        |   5 ++
 include/linux/bvec.h          |   6 ++
 11 files changed, 251 insertions(+), 165 deletions(-)

-- 
2.34.1

[PATCH 01/15] block: bio: rename page_is_mergeable to bio_page_is_mergeable and make non-static

[Jinyoung CHOI]

page_is_meargeable() can be used to merge the page to the bio_vec of
bio's integrity payload. For this, the static was removed.

There is a page_is_mergeable() in F2FS filesystem, so the name was changed
to bio_page_is_mergeable.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/bio.c         | 8 ++++----
 include/linux/bio.h | 3 +++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index fd11614bba4d..3e5ab59502e2 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -903,9 +903,9 @@ static inline bool bio_full(struct bio *bio, unsigned len)
 	return false;
 }
 
-static inline bool page_is_mergeable(const struct bio_vec *bv,
-		struct page *page, unsigned int len, unsigned int off,
-		bool *same_page)
+bool bio_page_is_mergeable(const struct bio_vec *bv, struct page *page,
+			   unsigned int len, unsigned int off,
+			   bool *same_page)
 {
 	size_t bv_end = bv->bv_offset + bv->bv_len;
 	phys_addr_t vec_end_addr = page_to_phys(bv->bv_page) + bv_end - 1;
@@ -951,7 +951,7 @@ static bool __bio_try_merge_page(struct bio *bio, struct page *page,
 	if (bio->bi_vcnt > 0) {
 		struct bio_vec *bv = &bio->bi_io_vec[bio->bi_vcnt - 1];
 
-		if (page_is_mergeable(bv, page, len, off, same_page)) {
+		if (bio_page_is_mergeable(bv, page, len, off, same_page)) {
 			if (bio->bi_iter.bi_size > UINT_MAX - len) {
 				*same_page = false;
 				return false;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index d766be7152e1..b53a595b519a 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -418,6 +418,9 @@ struct bio *bio_alloc_clone(struct block_device *bdev, struct bio *bio_src,
 		gfp_t gfp, struct bio_set *bs);
 int bio_init_clone(struct block_device *bdev, struct bio *bio,
 		struct bio *bio_src, gfp_t gfp);
+bool bio_page_is_mergeable(const struct bio_vec *bv, struct page *page,
+			   unsigned int len, unsigned int off,
+			   bool *same_page);
 
 extern struct bio_set fs_bio_set;
 
-- 
2.34.1

Re: [PATCH 01/15] block: bio: rename page_is_mergeable to bio_page_is_mergeable and make non-static

[hch]

> +bool bio_page_is_mergeable(const struct bio_vec *bv, struct page *page,
> +			   unsigned int len, unsigned int off,
> +			   bool *same_page)

No bio involved here, just a bvec.  But I don't really see the need
to rename it, just declare it in block/blk.h where f2fs has no chance
of ever seeing it.

RE:(2) [PATCH 01/15] block: bio: rename page_is_mergeable to bio_page_is_mergeable and make non-static

[Jinyoung CHOI]

>> +bool bio_page_is_mergeable(const struct bio_vec *bv, struct page *page,
>> +                           unsigned int len, unsigned int off,
>> +                           bool *same_page)
>
>No bio involved here, just a bvec.  But I don't really see the need
>to rename it, just declare it in block/blk.h where f2fs has no chance
>of ever seeing it.

Oh. You are right.
I will declare where you mentioned without changing the name.
It's better.

Best Regards,
Jinyoung

[PATCH 02/15] block: blk-integiry: add helper functions for bio_integrity_add_page

[Jinyoung CHOI]

Add functions that use the hardware limit to merge the page with
integrity's bio_vec.
Using the added functions, the physically continuous pages are made of
one bio_vec.

Previously, physically continuous pages were not created as one
bio_vec, but separately created.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/bio-integrity.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 4533eb491661..06b6a2c178d2 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -111,6 +111,64 @@ void bio_integrity_free(struct bio *bio)
 	bio->bi_opf &= ~REQ_INTEGRITY;
 }
 
+/**
+ * bip_full - check if the bip is full
+ * @bip:	bip to check
+ * @len:	length of one segment to be added
+ *
+ * Return true if @bip is full and one segment with @len bytes can't be
+ * added to the bip, otherwise return false
+ */
+static inline bool bip_full(struct bio_integrity_payload *bip, unsigned int len)
+{
+	if (bip->bip_vcnt >= bip->bip_max_vcnt)
+		return true;
+	if (bip->bip_iter.bi_size > UINT_MAX - len)
+		return true;
+	return false;
+}
+
+/**
+ * bip_try_merge_hw_seg - try to merge a page into a segment, while
+ * obeying the hardware segment size limit
+ * @q:		the target queue
+ * @bip:	bip to check
+ * @page:	page containing integrity metadata
+ * @len:	number of bytes of integrity metadata in page
+ * @offset:	start offset within page
+ * @same_page:  return if the segment has been merged inside the same page
+ *
+ * Return true if @page is merged to @bip, otherwise return false
+ */
+static bool bip_try_merge_hw_seg(struct request_queue *q,
+				 struct bio_integrity_payload *bip,
+				 struct page *page, unsigned int len,
+				 unsigned int offset, bool *same_page)
+{
+	struct bio_vec *bv = &bip->bip_vec[bip->bip_vcnt - 1];
+	unsigned long mask = queue_segment_boundary(q);
+	phys_addr_t addr1 = page_to_phys(bv->bv_page) + bv->bv_offset;
+	phys_addr_t addr2 = page_to_phys(page) + offset + len - 1;
+
+	if ((addr1 | mask) != (addr2 | mask))
+		return false;
+	if (bv->bv_len + len > queue_max_segment_size(q))
+		return false;
+
+	if (bip->bip_vcnt > 0) {
+		if (bio_page_is_mergeable(bv, page, len, offset, same_page)) {
+			if (bip->bip_iter.bi_size > UINT_MAX - len) {
+				*same_page = false;
+				return false;
+			}
+			bv->bv_len += len;
+			bip->bip_iter.bi_size += len;
+			return true;
+		}
+	}
+	return false;
+}
+
 /**
  * bio_integrity_add_page - Attach integrity metadata
  * @bio:	bio to update
-- 
2.34.1

Re: [PATCH 02/15] block: blk-integiry: add helper functions for bio_integrity_add_page

[hch]

s/blk-integiry/blk-integrity/ in the subject.

> +static inline bool bip_full(struct bio_integrity_payload *bip, unsigned int len)

> +static bool bip_try_merge_hw_seg(struct request_queue *q,
> +				 struct bio_integrity_payload *bip,
> +				 struct page *page, unsigned int len,
> +				 unsigned int offset, bool *same_page)

... but adding static functions without users will cause a compile
error anyway, so I'd suggest to just merge it into the patch adding
users.

But I wonder if we really want to duplicate all this logic anyway.
If we passed a bio_vec array, the vec count and an iter, we should
be able to just share the logic with the bio data payload.

RE:(2) [PATCH 02/15] block: blk-integiry: add helper functions for bio_integrity_add_page

[Jinyoung CHOI]

>s/blk-integiry/blk-integrity/ in the subject.
>
>> +static inline bool bip_full(struct bio_integrity_payload *bip, unsigned int len)
>
>> +static bool bip_try_merge_hw_seg(struct request_queue *q,
>> +                                 struct bio_integrity_payload *bip,
>> +                                 struct page *page, unsigned int len,
>> +                                 unsigned int offset, bool *same_page)
>
>... but adding static functions without users will cause a compile
>error anyway, so I'd suggest to just merge it into the patch adding
>users.
>
>But I wonder if we really want to duplicate all this logic anyway.
>If we passed a bio_vec array, the vec count and an iter, we should
>be able to just share the logic with the bio data payload.

Thank you, Christoph.

I made a mistake while dividing the patch. I will be careful.
I will merge with the code that uses the function and put it on the next patch.

And as you mentioned, it is duplicated with the configuration of bio's data payload.
I will improve this by reflecting your proposal.

Best regards,
Jinyoung.

[PATCH 03/15] block: bio-integrity: modify bio_integrity_add_page()

[Jinyoung CHOI]

Considering the constraints of hardware, the physically continuous pages
were to be composed of one bio_vec.

Previously, bio_vec was created separately for each page entering the
parameter.

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

Fixes: 783b94bd9250 ("nvme-pci: do not build a scatterlist to map metadata")
Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/bio-integrity.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 06b6a2c178d2..74cf9933c285 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -176,25 +176,45 @@ static bool bip_try_merge_hw_seg(struct request_queue *q,
  * @len:	number of bytes of integrity metadata in page
  * @offset:	start offset within page
  *
- * Description: Attach a page containing integrity metadata to bio.
+ * Add a page containing integrity metadata to a bio while respecting
+ * the hardware max_sectors, max_segment and gap limitations.
  */
 int bio_integrity_add_page(struct bio *bio, struct page *page,
 			   unsigned int len, unsigned int offset)
 {
+	struct request_queue *q = bdev_get_queue(bio->bi_bdev);
 	struct bio_integrity_payload *bip = bio_integrity(bio);
 
-	if (bip->bip_vcnt >= bip->bip_max_vcnt) {
+	if (((bip->bip_iter.bi_size + len) >> 9) > queue_max_hw_sectors(q))
+		return 0;
+
+	if (bip->bip_vcnt > 0) {
+		struct bio_vec *bv;
+		bool same_page = false;
+
+		if (bip_try_merge_hw_seg(q, bip, page, len, offset, &same_page))
+			return len;
+
+		/*
+		 * If the queue doesn't support SG gaps and adding this segment
+		 * would create a gap, disallow it.
+		 */
+		bv = &bip->bip_vec[bip->bip_vcnt - 1];
+		if (bvec_gap_to_prev(&q->limits, bv, offset))
+			return 0;
+	}
+
+	if (bip_full(bip, len)) {
 		printk(KERN_ERR "%s: bip_vec full\n", __func__);
 		return 0;
 	}
 
-	if (bip->bip_vcnt &&
-	    bvec_gap_to_prev(&bdev_get_queue(bio->bi_bdev)->limits,
-			     &bip->bip_vec[bip->bip_vcnt - 1], offset))
+	if (bip->bip_vcnt >= queue_max_integrity_segments(q))
 		return 0;
 
 	bvec_set_page(&bip->bip_vec[bip->bip_vcnt], page, len, offset);
 	bip->bip_vcnt++;
+	bip->bip_iter.bi_size += len;
 
 	return len;
 }
@@ -307,7 +327,6 @@ bool bio_integrity_prep(struct bio *bio)
 	}
 
 	bip->bip_flags |= BIP_BLOCK_INTEGRITY;
-	bip->bip_iter.bi_size = len;
 	bip_set_seed(bip, bio->bi_iter.bi_sector);
 
 	if (bi->flags & BLK_INTEGRITY_IP_CHECKSUM)
@@ -316,7 +335,6 @@ bool bio_integrity_prep(struct bio *bio)
 	/* Map it */
 	offset = offset_in_page(buf);
 	for (i = 0 ; i < nr_pages ; i++) {
-		int ret;
 		bytes = PAGE_SIZE - offset;
 
 		if (len <= 0)
@@ -325,18 +343,13 @@ bool bio_integrity_prep(struct bio *bio)
 		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");
 			status = BLK_STS_RESOURCE;
 			goto err_end_io;
 		}
 
-		if (ret < bytes)
-			break;
-
 		buf += bytes;
 		len -= bytes;
 		offset = 0;
-- 
2.34.1

[PATCH 04/15] block: bio-integiry: cleanup bio_integrity_prep

[Jinyoung CHOI]

If a problem occurs in the process of creating an integrity payload, the
status of bio is always BLK_STS_RESOURCE.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/bio-integrity.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 74cf9933c285..329c44eca83d 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -278,7 +278,6 @@ bool bio_integrity_prep(struct bio *bio)
 	unsigned int len, nr_pages;
 	unsigned int bytes, offset, i;
 	unsigned int intervals;
-	blk_status_t status;
 
 	if (!bi)
 		return true;
@@ -307,7 +306,6 @@ bool bio_integrity_prep(struct bio *bio)
 	/* Allocate kernel buffer for protection data */
 	len = intervals * bi->tuple_size;
 	buf = kmalloc(len, GFP_NOIO);
-	status = BLK_STS_RESOURCE;
 	if (unlikely(buf == NULL)) {
 		printk(KERN_ERR "could not allocate integrity buffer\n");
 		goto err_end_io;
@@ -322,7 +320,6 @@ bool bio_integrity_prep(struct bio *bio)
 	if (IS_ERR(bip)) {
 		printk(KERN_ERR "could not allocate data integrity bioset\n");
 		kfree(buf);
-		status = BLK_STS_RESOURCE;
 		goto err_end_io;
 	}
 
@@ -346,7 +343,6 @@ bool bio_integrity_prep(struct bio *bio)
 		if (bio_integrity_add_page(bio, virt_to_page(buf),
 					   bytes, offset) < bytes) {
 			printk(KERN_ERR "could not attach integrity payload\n");
-			status = BLK_STS_RESOURCE;
 			goto err_end_io;
 		}
 
@@ -365,10 +361,9 @@ bool bio_integrity_prep(struct bio *bio)
 	return true;
 
 err_end_io:
-	bio->bi_status = status;
+	bio->bi_status = BLK_STS_RESOURCE;
 	bio_endio(bio);
 	return false;
-
 }
 EXPORT_SYMBOL(bio_integrity_prep);
 
-- 
2.34.1

Re: [PATCH 04/15] block: bio-integiry: cleanup bio_integrity_prep

[hch]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 05/15] block: fix not to apply bip information in blk_rq_bio_prep()

[Jinyoung CHOI]

When a request is initialized through the bio, bio's integrity
information is not reflected in the request.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 include/linux/blk-mq.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 1dacb2c81fdd..cdb95e090919 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -961,6 +961,11 @@ static inline void blk_rq_bio_prep(struct request *rq, struct bio *bio,
 	rq->__data_len = bio->bi_iter.bi_size;
 	rq->bio = rq->biotail = bio;
 	rq->ioprio = bio_prio(bio);
+
+	if (bio_integrity(bio)) {
+		rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
+		rq->cmd_flags |= REQ_INTEGRITY;
+	}
 }
 
 void blk_mq_hctx_set_fq_lock_class(struct blk_mq_hw_ctx *hctx,
-- 
2.34.1

Re: [PATCH 05/15] block: fix not to apply bip information in blk_rq_bio_prep()

[kernel test robot]

Hi Jinyoung,

kernel test robot noticed the following build errors:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on mkp-scsi/for-next jejb-scsi/for-next linus/master v6.3 next-20230428]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20230503101048epcms2p61d61df1431955d9517c9939999ee3478%40epcms2p6
patch subject: [PATCH 05/15] block: fix not to apply bip information in blk_rq_bio_prep()
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20230503/202305032008.NxgqlW9X-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/584edc6ae9cb23e8a778ee73d711b9143038a047
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
        git checkout 584edc6ae9cb23e8a778ee73d711b9143038a047
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=um SUBARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash arch/um/drivers/ block/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305032008.NxgqlW9X-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from arch/um/drivers/ubd_kern.c:27:
   include/linux/blk-mq.h: In function 'blk_rq_bio_prep':
>> include/linux/blk-mq.h:972:19: error: 'struct request' has no member named 'nr_integrity_segments'
     972 |                 rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
         |                   ^~
>> include/linux/blk-mq.h:972:63: warning: dereferencing 'void *' pointer
     972 |                 rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
         |                                                               ^~
>> include/linux/blk-mq.h:972:63: error: request for member 'bip_vcnt' in something not a structure or union


vim +972 include/linux/blk-mq.h

   962	
   963	static inline void blk_rq_bio_prep(struct request *rq, struct bio *bio,
   964			unsigned int nr_segs)
   965	{
   966		rq->nr_phys_segments = nr_segs;
   967		rq->__data_len = bio->bi_iter.bi_size;
   968		rq->bio = rq->biotail = bio;
   969		rq->ioprio = bio_prio(bio);
   970	
   971		if (bio_integrity(bio)) {
 > 972			rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
   973			rq->cmd_flags |= REQ_INTEGRITY;
   974		}
   975	}
   976	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 05/15] block: fix not to apply bip information in blk_rq_bio_prep()

[hch]

On Wed, May 03, 2023 at 07:10:48PM +0900, Jinyoung CHOI wrote:
> When a request is initialized through the bio, bio's integrity
> information is not reflected in the request.

Uuh.  This looks like a pretty grave bug unless I'm missing something.
Can you:

 - submit this as a fix for 6.3 and -stable?
 - maybe find what broke this and add a fixes tag?

RE:(2) [PATCH 05/15] block: fix not to apply bip information in blk_rq_bio_prep()

[Jinyoung CHOI]

>> When a request is initialized through the bio, bio's integrity
>> information is not reflected in the request.
>
>Uuh.  This looks like a pretty grave bug unless I'm missing something.
>Can you:
>
> - submit this as a fix for 6.3 and -stable?
> - maybe find what broke this and add a fixes tag?

Thanks for your review.

OK, I will check it and proceed.

I didn't confirm that I turned off the config and compiled it.
I will check it and upload in the next patch. sorry.

Best Regards,
Jinyoung.

Re: [PATCH 05/15] block: fix not to apply bip information in blk_rq_bio_prep()

[kernel test robot]

Hi Jinyoung,

kernel test robot noticed the following build errors:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on mkp-scsi/for-next jejb-scsi/for-next linus/master v6.3 next-20230428]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20230503101048epcms2p61d61df1431955d9517c9939999ee3478%40epcms2p6
patch subject: [PATCH 05/15] block: fix not to apply bip information in blk_rq_bio_prep()
config: riscv-randconfig-r042-20230503 (https://download.01.org/0day-ci/archive/20230504/202305040157.ZWiorthB-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b1465cd49efcbc114a75220b153f5a055ce7911f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/584edc6ae9cb23e8a778ee73d711b9143038a047
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
        git checkout 584edc6ae9cb23e8a778ee73d711b9143038a047
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash block/ drivers/mtd/ubi/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305040157.ZWiorthB-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from block/bdev.c:15:
   In file included from include/linux/blk-integrity.h:5:
>> include/linux/blk-mq.h:972:7: error: no member named 'nr_integrity_segments' in 'struct request'
                   rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
                   ~~  ^
>> include/linux/blk-mq.h:972:49: error: member reference base type 'void' is not a structure or union
                   rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
                                               ~~~~~~~~~~~~~~~~~~^ ~~~~~~~~
   2 errors generated.


vim +972 include/linux/blk-mq.h

   962	
   963	static inline void blk_rq_bio_prep(struct request *rq, struct bio *bio,
   964			unsigned int nr_segs)
   965	{
   966		rq->nr_phys_segments = nr_segs;
   967		rq->__data_len = bio->bi_iter.bi_size;
   968		rq->bio = rq->biotail = bio;
   969		rq->ioprio = bio_prio(bio);
   970	
   971		if (bio_integrity(bio)) {
 > 972			rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
   973			rq->cmd_flags |= REQ_INTEGRITY;
   974		}
   975	}
   976	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 05/15] block: fix not to apply bip information in blk_rq_bio_prep()

[kernel test robot]

Hi Jinyoung,

kernel test robot noticed the following build warnings:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on mkp-scsi/for-next jejb-scsi/for-next linus/master v6.3 next-20230428]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20230503101048epcms2p61d61df1431955d9517c9939999ee3478%40epcms2p6
patch subject: [PATCH 05/15] block: fix not to apply bip information in blk_rq_bio_prep()
config: hexagon-randconfig-r045-20230503 (https://download.01.org/0day-ci/archive/20230504/202305040206.hTkXRgzQ-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project b1465cd49efcbc114a75220b153f5a055ce7911f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/584edc6ae9cb23e8a778ee73d711b9143038a047
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
        git checkout 584edc6ae9cb23e8a778ee73d711b9143038a047
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/md/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305040206.hTkXRgzQ-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/md/md-bitmap.c:19:
   In file included from include/linux/blkdev.h:9:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from drivers/md/md-bitmap.c:19:
   In file included from include/linux/blkdev.h:9:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from drivers/md/md-bitmap.c:19:
   In file included from include/linux/blkdev.h:9:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   In file included from drivers/md/md-bitmap.c:31:
   In file included from include/trace/events/block.h:8:
   In file included from include/linux/blktrace_api.h:5:
   include/linux/blk-mq.h:972:7: error: no member named 'nr_integrity_segments' in 'struct request'
                   rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
                   ~~  ^
   include/linux/blk-mq.h:972:49: error: member reference base type 'void' is not a structure or union
                   rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
                                               ~~~~~~~~~~~~~~~~~~^ ~~~~~~~~
>> drivers/md/md-bitmap.c:2564:34: warning: result of comparison of constant 4294967296 with expression of type 'unsigned long' is always false [-Wtautological-constant-out-of-range-compare]
           if (BITS_PER_LONG > 32 && csize >= (1ULL << (BITS_PER_BYTE *
                                     ~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
   7 warnings and 2 errors generated.


vim +2564 drivers/md/md-bitmap.c

43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2549  
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2550  static ssize_t
fd01b88c75a718 drivers/md/bitmap.c    NeilBrown             2011-10-11  2551  chunksize_store(struct mddev *mddev, const char *buf, size_t len)
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2552  {
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2553  	/* Can only be changed when no bitmap is active */
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2554  	int rv;
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2555  	unsigned long csize;
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2556  	if (mddev->bitmap)
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2557  		return -EBUSY;
b29bebd66dbd49 drivers/md/bitmap.c    Jingoo Han            2013-06-01  2558  	rv = kstrtoul(buf, 10, &csize);
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2559  	if (rv)
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2560  		return rv;
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2561  	if (csize < 512 ||
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2562  	    !is_power_of_2(csize))
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2563  		return -EINVAL;
4555211190798b drivers/md/md-bitmap.c Florian-Ewald Mueller 2022-10-25 @2564  	if (BITS_PER_LONG > 32 && csize >= (1ULL << (BITS_PER_BYTE *
4555211190798b drivers/md/md-bitmap.c Florian-Ewald Mueller 2022-10-25  2565  		sizeof(((bitmap_super_t *)0)->chunksize))))
4555211190798b drivers/md/md-bitmap.c Florian-Ewald Mueller 2022-10-25  2566  		return -EOVERFLOW;
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2567  	mddev->bitmap_info.chunksize = csize;
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2568  	return len;
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2569  }
43a705076e51c5 drivers/md/bitmap.c    NeilBrown             2009-12-14  2570  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH 06/15] block: blk-merge: fix to add the number of integrity segments to the request twice

[Jinyoung CHOI]

blk_integrity_merge_bio() not only performs conditional tests, but also
updates the integrity segment information of request.
It can be called twice when merging the bio into an existing request.

bio_attempt_bio_merge() or blk_mq_sched_try_merge()
  blk_rq_merge_ok()
    blk_integrity_merge_bio()  -  1
  bio_attemp_{back|front}_merge()
    ll_{back|front}_merge_fn()
      ll_new_hw_segments()
        blk_integrity_merge_bio()  -  2

The part of checking the conditions and the code to update the
information of the actual request were separated. At this time, the
ll_back_merge_fn was called by passth-path, so the condition check was
called by all the separated functions.

And after success in blk_integrity_merge_bio(), the information of the
request may be wrong if it is impossible to merge due to other
conditional tests. Thus, it was changed to be called immediately before
merging the bio's segments.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/blk-integrity.c | 34 +++++++++++++++++++++++++++++-----
 block/blk-merge.c     |  9 +++++----
 block/blk.h           |  7 +++++++
 3 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index d4e9b4556d14..03a85e1f6d2e 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -184,19 +184,43 @@ bool blk_integrity_merge_rq(struct request_queue *q, struct request *req,
 	return true;
 }
 
+static inline bool blk_integrity_bypass_check(struct request *req,
+					      struct bio *bio)
+{
+	return blk_integrity_rq(req) == 0 && bio_integrity(bio) == NULL;
+}
+
+static bool __blk_integrity_mergeable(struct request_queue *q,
+				      struct request *req, struct bio *bio)
+{
+	if (blk_integrity_rq(req) == 0 || bio_integrity(bio) == NULL)
+		return false;
+
+	if (bio_integrity(req->bio)->bip_flags != bio_integrity(bio)->bip_flags)
+		return false;
+
+	return true;
+}
+
+bool blk_integrity_mergeable(struct request_queue *q, struct request *req,
+			     struct bio *bio)
+{
+	if (blk_integrity_bypass_check(req, bio))
+		return true;
+
+	return __blk_integrity_mergeable(q, req, bio);
+}
+
 bool blk_integrity_merge_bio(struct request_queue *q, struct request *req,
 			     struct bio *bio)
 {
 	int nr_integrity_segs;
 	struct bio *next = bio->bi_next;
 
-	if (blk_integrity_rq(req) == 0 && bio_integrity(bio) == NULL)
+	if (blk_integrity_bypass_check(req, bio))
 		return true;
 
-	if (blk_integrity_rq(req) == 0 || bio_integrity(bio) == NULL)
-		return false;
-
-	if (bio_integrity(req->bio)->bip_flags != bio_integrity(bio)->bip_flags)
+	if (!__blk_integrity_mergeable(q, req, bio))
 		return false;
 
 	bio->bi_next = NULL;
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 65e75efa9bd3..8509f468d6d4 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -611,9 +611,6 @@ static inline int ll_new_hw_segment(struct request *req, struct bio *bio,
 	if (!blk_cgroup_mergeable(req, bio))
 		goto no_merge;
 
-	if (blk_integrity_merge_bio(req->q, req, bio) == false)
-		goto no_merge;
-
 	/* discard request merge won't add new segment */
 	if (req_op(req) == REQ_OP_DISCARD)
 		return 1;
@@ -621,6 +618,10 @@ static inline int ll_new_hw_segment(struct request *req, struct bio *bio,
 	if (req->nr_phys_segments + nr_phys_segs > blk_rq_get_max_segments(req))
 		goto no_merge;
 
+	/* This will merge integrity segments */
+	if (!blk_integrity_merge_bio(req->q, req, bio))
+		goto no_merge;
+
 	/*
 	 * This will form the start of a new hw segment.  Bump both
 	 * counters.
@@ -934,7 +935,7 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
 		return false;
 
 	/* only merge integrity protected bio into ditto rq */
-	if (blk_integrity_merge_bio(rq->q, rq, bio) == false)
+	if (!blk_integrity_mergeable(rq->q, rq, bio))
 		return false;
 
 	/* Only merge if the crypt contexts are compatible */
diff --git a/block/blk.h b/block/blk.h
index 45547bcf1119..5923d2190d91 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -189,6 +189,8 @@ static inline bool bio_integrity_endio(struct bio *bio)
 
 bool blk_integrity_merge_rq(struct request_queue *, struct request *,
 		struct request *);
+bool blk_integrity_mergeable(struct request_queue *rq, struct request *r,
+		struct bio *b);
 bool blk_integrity_merge_bio(struct request_queue *, struct request *,
 		struct bio *);
 
@@ -221,6 +223,11 @@ static inline bool blk_integrity_merge_rq(struct request_queue *rq,
 {
 	return true;
 }
+static inline bool blk_integrity_mergeable(struct request_queue *rq,
+		struct request *r, struct bio *b)
+{
+	return true;
+}
 static inline bool blk_integrity_merge_bio(struct request_queue *rq,
 		struct request *r, struct bio *b)
 {
-- 
2.34.1

[PATCH 07/15] block: blk-merge: fix merging two requests in ll_merge_requests_fn

[Jinyoung CHOI]

blk_integrity_merge_rq() merges integrity segment information of two
requests. However, it is only a condition check and does not perform the
actual integrity information update. So this was modified.

After it is called, the merge process of the requests may fail
due to other conditions. At this time, there is an error in the integrity
segment information of request. So the call location was also changed.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/blk-integrity.c | 2 ++
 block/blk-merge.c     | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 03a85e1f6d2e..f97b7e8a6d4d 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -181,6 +181,8 @@ bool blk_integrity_merge_rq(struct request_queue *q, struct request *req,
 	if (integrity_req_gap_back_merge(req, next->bio))
 		return false;
 
+	req->nr_integrity_segments += next->nr_integrity_segments;
+
 	return true;
 }
 
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 8509f468d6d4..c6a0958e8df1 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -711,10 +711,11 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req,
 	if (!blk_cgroup_mergeable(req, next->bio))
 		return 0;
 
-	if (blk_integrity_merge_rq(q, req, next) == false)
+	if (!bio_crypt_ctx_merge_rq(req, next))
 		return 0;
 
-	if (!bio_crypt_ctx_merge_rq(req, next))
+	/* this will merge integrity segments */
+	if (!blk_integrity_merge_rq(q, req, next))
 		return 0;
 
 	/* Merge is OK... */
-- 
2.34.1

[PATCH 08/15] block: add helper function to get the number of integrity segments

[Jinyoung CHOI]

Since request always has the number of integrity segments in the process
of generating and merging, a function for simply obtained this has been
added.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 include/linux/blk-integrity.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/blk-integrity.h b/include/linux/blk-integrity.h
index 378b2459efe2..45b9fde1fee1 100644
--- a/include/linux/blk-integrity.h
+++ b/include/linux/blk-integrity.h
@@ -43,6 +43,11 @@ int blk_rq_map_integrity_sg(struct request_queue *, struct bio *,
 				   struct scatterlist *);
 int blk_rq_count_integrity_sg(struct request_queue *, struct bio *);
 
+static inline unsigned short blk_rq_nr_integrity_segments(struct request *rq)
+{
+	return rq->nr_integrity_segments;
+}
+
 static inline struct blk_integrity *blk_get_integrity(struct gendisk *disk)
 {
 	struct blk_integrity *bi = &disk->queue->integrity;
@@ -120,6 +125,10 @@ static inline int blk_rq_count_integrity_sg(struct request_queue *q,
 {
 	return 0;
 }
+static inline unsigned short blk_rq_nr_integrity_segments(struct request *rq)
+{
+	return 0;
+}
 static inline int blk_rq_map_integrity_sg(struct request_queue *q,
 					  struct bio *b,
 					  struct scatterlist *s)
-- 
2.34.1

[PATCH 09/15] scsi: add scsi_alloc_integrity_sgtables() for integrity process

[Jinyoung CHOI]

Separate the integrity mapping process of scsi_alloc_sgtables() into a
new function for readability.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 drivers/scsi/scsi_lib.c | 71 ++++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b7c569a42aa4..89cf21345e1a 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1003,6 +1003,40 @@ static inline bool scsi_cmd_needs_dma_drain(struct scsi_device *sdev,
 	       sdev->host->hostt->dma_need_drain(rq);
 }
 
+static blk_status_t scsi_alloc_integrity_sgtables(struct scsi_cmnd *cmd)
+{
+	struct request *rq = scsi_cmd_to_rq(cmd);
+	struct scsi_data_buffer *prot_sdb = cmd->prot_sdb;
+	int count, ivecs;
+
+	if (WARN_ON_ONCE(!prot_sdb)) {
+		/*
+		 * This can happen if someone (e.g. multipath)
+		 * queues a command to a device on an adapter
+		 * that does not support DIX.
+		 */
+		return BLK_STS_IOERR;
+	}
+
+	ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
+
+	if (sg_alloc_table_chained(&prot_sdb->table, ivecs,
+				   prot_sdb->table.sgl,
+				   SCSI_INLINE_PROT_SG_CNT)) {
+		return BLK_STS_RESOURCE;
+	}
+
+	count = blk_rq_map_integrity_sg(rq->q, rq->bio, prot_sdb->table.sgl);
+
+	BUG_ON(count > ivecs);
+	BUG_ON(count > queue_max_integrity_segments(rq->q));
+
+	cmd->prot_sdb = prot_sdb;
+	cmd->prot_sdb->table.nents = count;
+
+	return BLK_STS_OK;
+}
+
 /**
  * scsi_alloc_sgtables - Allocate and initialize data and integrity scatterlists
  * @cmd: SCSI command data structure to initialize.
@@ -1021,7 +1055,7 @@ blk_status_t scsi_alloc_sgtables(struct scsi_cmnd *cmd)
 	struct request *rq = scsi_cmd_to_rq(cmd);
 	unsigned short nr_segs = blk_rq_nr_phys_segments(rq);
 	struct scatterlist *last_sg = NULL;
-	blk_status_t ret;
+	blk_status_t ret = BLK_STS_OK;
 	bool need_drain = scsi_cmd_needs_dma_drain(sdev, rq);
 	int count;
 
@@ -1071,40 +1105,11 @@ blk_status_t scsi_alloc_sgtables(struct scsi_cmnd *cmd)
 	cmd->sdb.length = blk_rq_payload_bytes(rq);
 
 	if (blk_integrity_rq(rq)) {
-		struct scsi_data_buffer *prot_sdb = cmd->prot_sdb;
-		int ivecs;
-
-		if (WARN_ON_ONCE(!prot_sdb)) {
-			/*
-			 * This can happen if someone (e.g. multipath)
-			 * queues a command to a device on an adapter
-			 * that does not support DIX.
-			 */
-			ret = BLK_STS_IOERR;
-			goto out_free_sgtables;
-		}
-
-		ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
-
-		if (sg_alloc_table_chained(&prot_sdb->table, ivecs,
-				prot_sdb->table.sgl,
-				SCSI_INLINE_PROT_SG_CNT)) {
-			ret = BLK_STS_RESOURCE;
-			goto out_free_sgtables;
-		}
-
-		count = blk_rq_map_integrity_sg(rq->q, rq->bio,
-						prot_sdb->table.sgl);
-		BUG_ON(count > ivecs);
-		BUG_ON(count > queue_max_integrity_segments(rq->q));
-
-		cmd->prot_sdb = prot_sdb;
-		cmd->prot_sdb->table.nents = count;
+		ret = scsi_alloc_integrity_sgtables(cmd);
+		if (ret != BLK_STS_OK)
+			scsi_free_sgtables(cmd);
 	}
 
-	return BLK_STS_OK;
-out_free_sgtables:
-	scsi_free_sgtables(cmd);
 	return ret;
 }
 EXPORT_SYMBOL(scsi_alloc_sgtables);
-- 
2.34.1

[PATCH 10/15] scsi: change to use blk_rq_nr_integrity_segments() instead of blk_rq_count_integrity_sg()

[Jinyoung CHOI]

In the block layer, the number of segments is recorded in the request.
It is already divided into separate segments based on hw information.
Therefore, there is no need to count again.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 drivers/scsi/scsi_lib.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 89cf21345e1a..5d67b6f6854e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1006,8 +1006,9 @@ static inline bool scsi_cmd_needs_dma_drain(struct scsi_device *sdev,
 static blk_status_t scsi_alloc_integrity_sgtables(struct scsi_cmnd *cmd)
 {
 	struct request *rq = scsi_cmd_to_rq(cmd);
+	unsigned short nr_integrity_segs = blk_rq_nr_integrity_segments(rq);
 	struct scsi_data_buffer *prot_sdb = cmd->prot_sdb;
-	int count, ivecs;
+	int count;
 
 	if (WARN_ON_ONCE(!prot_sdb)) {
 		/*
@@ -1018,9 +1019,7 @@ static blk_status_t scsi_alloc_integrity_sgtables(struct scsi_cmnd *cmd)
 		return BLK_STS_IOERR;
 	}
 
-	ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
-
-	if (sg_alloc_table_chained(&prot_sdb->table, ivecs,
+	if (sg_alloc_table_chained(&prot_sdb->table, nr_integrity_segs,
 				   prot_sdb->table.sgl,
 				   SCSI_INLINE_PROT_SG_CNT)) {
 		return BLK_STS_RESOURCE;
@@ -1028,10 +1027,7 @@ static blk_status_t scsi_alloc_integrity_sgtables(struct scsi_cmnd *cmd)
 
 	count = blk_rq_map_integrity_sg(rq->q, rq->bio, prot_sdb->table.sgl);
 
-	BUG_ON(count > ivecs);
-	BUG_ON(count > queue_max_integrity_segments(rq->q));
-
-	cmd->prot_sdb = prot_sdb;
+	BUG_ON(count > prot_sdb->table.nents);
 	cmd->prot_sdb->table.nents = count;
 
 	return BLK_STS_OK;
-- 
2.34.1

[PATCH 11/15] block: blk-integrity: change how to find the number of integrity of bio

[Jinyoung CHOI]

The method of constructing a bip has been changed, the number of
segments can be obtained through bip_vcnt.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/blk-integrity.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index f97b7e8a6d4d..64407b412947 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -217,7 +217,6 @@ bool blk_integrity_merge_bio(struct request_queue *q, struct request *req,
 			     struct bio *bio)
 {
 	int nr_integrity_segs;
-	struct bio *next = bio->bi_next;
 
 	if (blk_integrity_bypass_check(req, bio))
 		return true;
@@ -225,9 +224,7 @@ bool blk_integrity_merge_bio(struct request_queue *q, struct request *req,
 	if (!__blk_integrity_mergeable(q, req, bio))
 		return false;
 
-	bio->bi_next = NULL;
-	nr_integrity_segs = blk_rq_count_integrity_sg(q, bio);
-	bio->bi_next = next;
+	nr_integrity_segs = bio_integrity(bio)->bip_vcnt;
 
 	if (req->nr_integrity_segments + nr_integrity_segs >
 	    q->limits.max_integrity_segments)
-- 
2.34.1

[PATCH 12/15] nvme: rdma: change how to find the number of integrity of request

[Jinyoung CHOI]

Since the request has the number of integrity segments, change to use
the relevant api.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 drivers/nvme/host/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 0eb79696fb73..237d81ad54af 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1507,7 +1507,7 @@ static int nvme_rdma_dma_map_req(struct ib_device *ibdev, struct request *rq,
 		req->metadata_sgl->sg_table.sgl =
 			(struct scatterlist *)(req->metadata_sgl + 1);
 		ret = sg_alloc_table_chained(&req->metadata_sgl->sg_table,
-				blk_rq_count_integrity_sg(rq->q, rq->bio),
+				blk_rq_nr_integrity_segments(rq),
 				req->metadata_sgl->sg_table.sgl,
 				NVME_INLINE_METADATA_SG_CNT);
 		if (unlikely(ret)) {
-- 
2.34.1

[PATCH 13/15] block: add helper function for iteration of bip's bvec

[Jinyoung CHOI]

bip_for_each_vec() performs the iteration in a page unit.

Since a bio_vec of bip is composed of multi-page in the block, a macro
that can be carried out in multi-page units has been added.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 include/linux/bio.h  | 4 ++++
 include/linux/bvec.h | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/include/linux/bio.h b/include/linux/bio.h
index b53a595b519a..e3e437ce694c 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -695,6 +695,10 @@ static inline bool bioset_initialized(struct bio_set *bs)
 
 #if defined(CONFIG_BLK_DEV_INTEGRITY)
 
+/* iterate over multi-page bvec for integrity */
+#define bip_for_each_mp_bvec(bvl, bip, iter)				\
+	for_each_mp_bvec(bvl, (bip)->bip_vec, iter, (bip)->bip_iter)
+
 #define bip_for_each_vec(bvl, bip, iter)				\
 	for_each_bvec(bvl, (bip)->bip_vec, iter, (bip)->bip_iter)
 
diff --git a/include/linux/bvec.h b/include/linux/bvec.h
index 555aae5448ae..9364c258513e 100644
--- a/include/linux/bvec.h
+++ b/include/linux/bvec.h
@@ -184,6 +184,12 @@ static inline void bvec_iter_advance_single(const struct bio_vec *bv,
 		((bvl = bvec_iter_bvec((bio_vec), (iter))), 1);	\
 	     bvec_iter_advance_single((bio_vec), &(iter), (bvl).bv_len))
 
+#define for_each_mp_bvec(bvl, bio_vec, iter, start)			\
+	for (iter = (start);						\
+	     (iter).bi_size &&						\
+		((bvl = mp_bvec_iter_bvec((bio_vec), (iter))), 1);	\
+	     bvec_iter_advance_single((bio_vec), &(iter), (bvl).bv_len))
+
 /* for iterating one bio from start to end */
 #define BVEC_ITER_ALL_INIT (struct bvec_iter)				\
 {									\
-- 
2.34.1

[PATCH 14/15] block: blk-integrity: change sg-table configuration method for integrity

[Jinyoung CHOI]

Previously, a bio_vec of bip was made of one page in the block layer, and
sg_list was generated using hw information in lld.

This is done in the block layer and the bio_vec has been changed to
multi-page, so it is changed to configure the sg-table using the existing
api, such as the sg-table setting for the bio. (e.g. multi-page map)

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/blk-integrity.c | 52 -------------------------------------------
 block/blk-merge.c     | 52 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 64407b412947..c50954652177 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -55,58 +55,6 @@ int blk_rq_count_integrity_sg(struct request_queue *q, struct bio *bio)
 }
 EXPORT_SYMBOL(blk_rq_count_integrity_sg);
 
-/**
- * blk_rq_map_integrity_sg - Map integrity metadata into a scatterlist
- * @q:		request queue
- * @bio:	bio with integrity metadata attached
- * @sglist:	target scatterlist
- *
- * Description: Map the integrity vectors in request into a
- * scatterlist.  The scatterlist must be big enough to hold all
- * elements.  I.e. sized using blk_rq_count_integrity_sg().
- */
-int blk_rq_map_integrity_sg(struct request_queue *q, struct bio *bio,
-			    struct scatterlist *sglist)
-{
-	struct bio_vec iv, ivprv = { NULL };
-	struct scatterlist *sg = NULL;
-	unsigned int segments = 0;
-	struct bvec_iter iter;
-	int prev = 0;
-
-	bio_for_each_integrity_vec(iv, bio, iter) {
-
-		if (prev) {
-			if (!biovec_phys_mergeable(q, &ivprv, &iv))
-				goto new_segment;
-			if (sg->length + iv.bv_len > queue_max_segment_size(q))
-				goto new_segment;
-
-			sg->length += iv.bv_len;
-		} else {
-new_segment:
-			if (!sg)
-				sg = sglist;
-			else {
-				sg_unmark_end(sg);
-				sg = sg_next(sg);
-			}
-
-			sg_set_page(sg, iv.bv_page, iv.bv_len, iv.bv_offset);
-			segments++;
-		}
-
-		prev = 1;
-		ivprv = iv;
-	}
-
-	if (sg)
-		sg_mark_end(sg);
-
-	return segments;
-}
-EXPORT_SYMBOL(blk_rq_map_integrity_sg);
-
 /**
  * blk_integrity_compare - Compare integrity profile of two disks
  * @gd1:	Disk to compare
diff --git a/block/blk-merge.c b/block/blk-merge.c
index c6a0958e8df1..71539d88ffe6 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -524,6 +524,58 @@ __blk_segment_map_sg_merge(struct request_queue *q, struct bio_vec *bvec,
 	return true;
 }
 
+/**
+ * blk_rq_map_integrity_sg - Map integrity metadata into a scatterlist
+ * @q:		request queue
+ * @bio:	bio with integrity metadata attached
+ * @sglist:	target scatterlist
+ *
+ * Description: Map the integrity vectors in request into a scatterlist.
+ * The scatterlist must be big enough to hold all elements.
+ */
+int blk_rq_map_integrity_sg(struct request_queue *q, struct bio *bio,
+			    struct scatterlist *sglist)
+{
+	struct bio_vec iv, ivprv = { NULL };
+	struct scatterlist *sg = NULL;
+	unsigned int nsegs = 0;
+	struct bvec_iter iter;
+	bool new_bio = false;
+
+	for_each_bio(bio) {
+		struct bio_integrity_payload *bip = bio->bi_integrity;
+
+		bip_for_each_mp_bvec(iv, bip, iter) {
+			/*
+			 * Only try to merge bvecs from two bios given we
+			 * have done bio internal merge when adding pages
+			 * to bio
+			 */
+			if (new_bio &&
+			    __blk_segment_map_sg_merge(q, &iv, &ivprv, &sg))
+				goto next_iv;
+
+			if (iv.bv_offset + iv.bv_len <= PAGE_SIZE)
+				nsegs += __blk_bvec_map_sg(iv, sglist, &sg);
+			else
+				nsegs += blk_bvec_map_sg(q, &iv, sglist, &sg);
+ next_iv:
+			new_bio = false;
+		}
+
+		if (likely(bip->bip_iter.bi_size)) {
+			ivprv = iv;
+			new_bio = true;
+		}
+	}
+
+	if (sg)
+		sg_mark_end(sg);
+
+	return nsegs;
+}
+EXPORT_SYMBOL(blk_rq_map_integrity_sg);
+
 static int __blk_bios_map_sg(struct request_queue *q, struct bio *bio,
 			     struct scatterlist *sglist,
 			     struct scatterlist **sg)
-- 
2.34.1

Re: [PATCH 14/15] block: blk-integrity: change sg-table configuration method for integrity

[kernel test robot]

Hi Jinyoung,

kernel test robot noticed the following build errors:

[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on mkp-scsi/for-next jejb-scsi/for-next linus/master v6.3 next-20230428]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20230503102719epcms2p457434fefd535ee43d502eff854227919%40epcms2p4
patch subject: [PATCH 14/15] block: blk-integrity: change sg-table configuration method for integrity
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20230503/202305032213.JHLycY4F-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/7e4810d74ca92ed35bbc73cb09a4baa1dacfbc96
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
        git checkout 7e4810d74ca92ed35bbc73cb09a4baa1dacfbc96
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=um SUBARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305032213.JHLycY4F-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/blk-integrity.h:5,
                    from block/blk-merge.c:9:
   include/linux/blk-mq.h: In function 'blk_rq_bio_prep':
   include/linux/blk-mq.h:972:19: error: 'struct request' has no member named 'nr_integrity_segments'
     972 |                 rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
         |                   ^~
   include/linux/blk-mq.h:972:63: warning: dereferencing 'void *' pointer
     972 |                 rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
         |                                                               ^~
   include/linux/blk-mq.h:972:63: error: request for member 'bip_vcnt' in something not a structure or union
   block/blk-merge.c: At top level:
>> block/blk-merge.c:536:5: error: redefinition of 'blk_rq_map_integrity_sg'
     536 | int blk_rq_map_integrity_sg(struct request_queue *q, struct bio *bio,
         |     ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from block/blk-merge.c:9:
   include/linux/blk-integrity.h:132:19: note: previous definition of 'blk_rq_map_integrity_sg' with type 'int(struct request_queue *, struct bio *, struct scatterlist *)'
     132 | static inline int blk_rq_map_integrity_sg(struct request_queue *q,
         |                   ^~~~~~~~~~~~~~~~~~~~~~~
   block/blk-merge.c: In function 'blk_rq_map_integrity_sg':
>> block/blk-merge.c:546:56: error: 'struct bio' has no member named 'bi_integrity'
     546 |                 struct bio_integrity_payload *bip = bio->bi_integrity;
         |                                                        ^~
>> block/blk-merge.c:548:17: error: implicit declaration of function 'bip_for_each_mp_bvec'; did you mean 'for_each_mp_bvec'? [-Werror=implicit-function-declaration]
     548 |                 bip_for_each_mp_bvec(iv, bip, iter) {
         |                 ^~~~~~~~~~~~~~~~~~~~
         |                 for_each_mp_bvec
>> block/blk-merge.c:548:52: error: expected ';' before '{' token
     548 |                 bip_for_each_mp_bvec(iv, bip, iter) {
         |                                                    ^~
         |                                                    ;
   block/blk-merge.c:543:14: warning: unused variable 'new_bio' [-Wunused-variable]
     543 |         bool new_bio = false;
         |              ^~~~~~~
   block/blk-merge.c:539:28: warning: unused variable 'ivprv' [-Wunused-variable]
     539 |         struct bio_vec iv, ivprv = { NULL };
         |                            ^~~~~
   cc1: some warnings being treated as errors


vim +/blk_rq_map_integrity_sg +536 block/blk-merge.c

   526	
   527	/**
   528	 * blk_rq_map_integrity_sg - Map integrity metadata into a scatterlist
   529	 * @q:		request queue
   530	 * @bio:	bio with integrity metadata attached
   531	 * @sglist:	target scatterlist
   532	 *
   533	 * Description: Map the integrity vectors in request into a scatterlist.
   534	 * The scatterlist must be big enough to hold all elements.
   535	 */
 > 536	int blk_rq_map_integrity_sg(struct request_queue *q, struct bio *bio,
   537				    struct scatterlist *sglist)
   538	{
   539		struct bio_vec iv, ivprv = { NULL };
   540		struct scatterlist *sg = NULL;
   541		unsigned int nsegs = 0;
   542		struct bvec_iter iter;
   543		bool new_bio = false;
   544	
   545		for_each_bio(bio) {
 > 546			struct bio_integrity_payload *bip = bio->bi_integrity;
   547	
 > 548			bip_for_each_mp_bvec(iv, bip, iter) {
   549				/*
   550				 * Only try to merge bvecs from two bios given we
   551				 * have done bio internal merge when adding pages
   552				 * to bio
   553				 */
   554				if (new_bio &&
   555				    __blk_segment_map_sg_merge(q, &iv, &ivprv, &sg))
   556					goto next_iv;
   557	
   558				if (iv.bv_offset + iv.bv_len <= PAGE_SIZE)
   559					nsegs += __blk_bvec_map_sg(iv, sglist, &sg);
   560				else
   561					nsegs += blk_bvec_map_sg(q, &iv, sglist, &sg);
   562	 next_iv:
   563				new_bio = false;
   564			}
   565	
   566			if (likely(bip->bip_iter.bi_size)) {
   567				ivprv = iv;
   568				new_bio = true;
   569			}
   570		}
   571	
   572		if (sg)
   573			sg_mark_end(sg);
   574	
   575		return nsegs;
   576	}
   577	EXPORT_SYMBOL(blk_rq_map_integrity_sg);
   578	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH 14/15] block: blk-integrity: change sg-table configuration method for integrity

[kernel test robot]

Hi Jinyoung,

kernel test robot noticed the following build warnings:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on mkp-scsi/for-next jejb-scsi/for-next linus/master v6.3 next-20230428]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20230503102719epcms2p457434fefd535ee43d502eff854227919%40epcms2p4
patch subject: [PATCH 14/15] block: blk-integrity: change sg-table configuration method for integrity
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20230504/202305040333.EHY8r49K-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/7e4810d74ca92ed35bbc73cb09a4baa1dacfbc96
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jinyoung-CHOI/block-blk-integiry-add-helper-functions-for-bio_integrity_add_page/20230503-183015
        git checkout 7e4810d74ca92ed35bbc73cb09a4baa1dacfbc96
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305040333.EHY8r49K-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/blk-integrity.h:5,
                    from block/blk-merge.c:9:
   include/linux/blk-mq.h: In function 'blk_rq_bio_prep':
   include/linux/blk-mq.h:972:19: error: 'struct request' has no member named 'nr_integrity_segments'
     972 |                 rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
         |                   ^~
   include/linux/blk-mq.h:972:63: warning: dereferencing 'void *' pointer
     972 |                 rq->nr_integrity_segments = bio_integrity(bio)->bip_vcnt;
         |                                                               ^~
   include/linux/blk-mq.h:972:63: error: request for member 'bip_vcnt' in something not a structure or union
   block/blk-merge.c: At top level:
   block/blk-merge.c:536:5: error: redefinition of 'blk_rq_map_integrity_sg'
     536 | int blk_rq_map_integrity_sg(struct request_queue *q, struct bio *bio,
         |     ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from block/blk-merge.c:9:
   include/linux/blk-integrity.h:132:19: note: previous definition of 'blk_rq_map_integrity_sg' with type 'int(struct request_queue *, struct bio *, struct scatterlist *)'
     132 | static inline int blk_rq_map_integrity_sg(struct request_queue *q,
         |                   ^~~~~~~~~~~~~~~~~~~~~~~
   block/blk-merge.c: In function 'blk_rq_map_integrity_sg':
   block/blk-merge.c:546:56: error: 'struct bio' has no member named 'bi_integrity'
     546 |                 struct bio_integrity_payload *bip = bio->bi_integrity;
         |                                                        ^~
   block/blk-merge.c:548:17: error: implicit declaration of function 'bip_for_each_mp_bvec'; did you mean 'for_each_mp_bvec'? [-Werror=implicit-function-declaration]
     548 |                 bip_for_each_mp_bvec(iv, bip, iter) {
         |                 ^~~~~~~~~~~~~~~~~~~~
         |                 for_each_mp_bvec
   block/blk-merge.c:548:52: error: expected ';' before '{' token
     548 |                 bip_for_each_mp_bvec(iv, bip, iter) {
         |                                                    ^~
         |                                                    ;
>> block/blk-merge.c:543:14: warning: unused variable 'new_bio' [-Wunused-variable]
     543 |         bool new_bio = false;
         |              ^~~~~~~
>> block/blk-merge.c:539:28: warning: unused variable 'ivprv' [-Wunused-variable]
     539 |         struct bio_vec iv, ivprv = { NULL };
         |                            ^~~~~
   cc1: some warnings being treated as errors


vim +/new_bio +543 block/blk-merge.c

   526	
   527	/**
   528	 * blk_rq_map_integrity_sg - Map integrity metadata into a scatterlist
   529	 * @q:		request queue
   530	 * @bio:	bio with integrity metadata attached
   531	 * @sglist:	target scatterlist
   532	 *
   533	 * Description: Map the integrity vectors in request into a scatterlist.
   534	 * The scatterlist must be big enough to hold all elements.
   535	 */
   536	int blk_rq_map_integrity_sg(struct request_queue *q, struct bio *bio,
   537				    struct scatterlist *sglist)
   538	{
 > 539		struct bio_vec iv, ivprv = { NULL };
   540		struct scatterlist *sg = NULL;
   541		unsigned int nsegs = 0;
   542		struct bvec_iter iter;
 > 543		bool new_bio = false;
   544	
   545		for_each_bio(bio) {
   546			struct bio_integrity_payload *bip = bio->bi_integrity;
   547	
   548			bip_for_each_mp_bvec(iv, bip, iter) {
   549				/*
   550				 * Only try to merge bvecs from two bios given we
   551				 * have done bio internal merge when adding pages
   552				 * to bio
   553				 */
   554				if (new_bio &&
   555				    __blk_segment_map_sg_merge(q, &iv, &ivprv, &sg))
   556					goto next_iv;
   557	
   558				if (iv.bv_offset + iv.bv_len <= PAGE_SIZE)
   559					nsegs += __blk_bvec_map_sg(iv, sglist, &sg);
   560				else
   561					nsegs += blk_bvec_map_sg(q, &iv, sglist, &sg);
   562	 next_iv:
   563				new_bio = false;
   564			}
   565	
   566			if (likely(bip->bip_iter.bi_size)) {
   567				ivprv = iv;
   568				new_bio = true;
   569			}
   570		}
   571	
   572		if (sg)
   573			sg_mark_end(sg);
   574	
   575		return nsegs;
   576	}
   577	EXPORT_SYMBOL(blk_rq_map_integrity_sg);
   578	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH 15/15] block: blk-integrity: remove blk_rq_count_integrity_sg()

[Jinyoung CHOI]

blk_rq_nr_nr_integrity_segments() allows you to obtain the number of
integrity. Therefore, blk_rq_count_integrity_sg() is no longer necessary.

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

Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com>
---
 block/blk-integrity.c         | 39 -----------------------------------
 include/linux/blk-integrity.h |  1 -
 2 files changed, 40 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index c50954652177..9bac2836c3ff 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -16,45 +16,6 @@
 
 #include "blk.h"
 
-/**
- * blk_rq_count_integrity_sg - Count number of integrity scatterlist elements
- * @q:		request queue
- * @bio:	bio with integrity metadata attached
- *
- * Description: Returns the number of elements required in a
- * scatterlist corresponding to the integrity metadata in a bio.
- */
-int blk_rq_count_integrity_sg(struct request_queue *q, struct bio *bio)
-{
-	struct bio_vec iv, ivprv = { NULL };
-	unsigned int segments = 0;
-	unsigned int seg_size = 0;
-	struct bvec_iter iter;
-	int prev = 0;
-
-	bio_for_each_integrity_vec(iv, bio, iter) {
-
-		if (prev) {
-			if (!biovec_phys_mergeable(q, &ivprv, &iv))
-				goto new_segment;
-			if (seg_size + iv.bv_len > queue_max_segment_size(q))
-				goto new_segment;
-
-			seg_size += iv.bv_len;
-		} else {
-new_segment:
-			segments++;
-			seg_size = iv.bv_len;
-		}
-
-		prev = 1;
-		ivprv = iv;
-	}
-
-	return segments;
-}
-EXPORT_SYMBOL(blk_rq_count_integrity_sg);
-
 /**
  * blk_integrity_compare - Compare integrity profile of two disks
  * @gd1:	Disk to compare
diff --git a/include/linux/blk-integrity.h b/include/linux/blk-integrity.h
index 45b9fde1fee1..a2a9d72e8fab 100644
--- a/include/linux/blk-integrity.h
+++ b/include/linux/blk-integrity.h
@@ -41,7 +41,6 @@ void blk_integrity_unregister(struct gendisk *);
 int blk_integrity_compare(struct gendisk *, struct gendisk *);
 int blk_rq_map_integrity_sg(struct request_queue *, struct bio *,
 				   struct scatterlist *);
-int blk_rq_count_integrity_sg(struct request_queue *, struct bio *);
 
 static inline unsigned short blk_rq_nr_integrity_segments(struct request *rq)
 {
-- 
2.34.1