2023-05-03 09:53:18

by Jinyoung Choi

[permalink] [raw]
Subject: [PATCH 00/15] Change the integrity configuration method in block

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


2023-05-03 10:10:00

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:15:26

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:19:09

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Fixes: 783b94bd9250 ("nvme-pci: do not build a scatterlist to map metadata")
Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:20:08

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:20:56

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:22:04

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:22:14

by Jinyoung Choi

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

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

Cc: Christoph Hellwig <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:26:25

by Jinyoung Choi

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

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

Cc: Christoph Hellwig <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:27:34

by Jinyoung Choi

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

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

Cc: Christoph Hellwig <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:28:42

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:31:45

by Jinyoung Choi

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

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

Cc: Christoph Hellwig <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:32:26

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:36:16

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:39:23

by Jinyoung Choi

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

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 <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 10:40:14

by Jinyoung Choi

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

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

Cc: Christoph Hellwig <[email protected]>
Cc: Martin K. Petersen <[email protected]>

Signed-off-by: Jinyoung Choi <[email protected]>
---
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

2023-05-03 12:58:21

by kernel test robot

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

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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2023-05-03 14:33:48

by kernel test robot

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

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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2023-05-03 16:09:01

by Christoph Hellwig

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

> +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.

2023-05-03 16:09:11

by Christoph Hellwig

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

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.

2023-05-03 16:09:16

by Christoph Hellwig

[permalink] [raw]

2023-05-03 16:09:49

by Christoph Hellwig

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

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?

2023-05-03 17:44:22

by kernel test robot

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

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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2023-05-03 19:11:09

by kernel test robot

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

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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2023-05-03 20:08:56

by kernel test robot

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

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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2023-05-04 06:24:45

by Jinyoung Choi

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

>> 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.

2023-05-04 06:38:40

by Jinyoung Choi

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

>> +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

2023-05-04 06:54:08

by Jinyoung Choi

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

>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.