Skip to content

Commit 9bce6b5

Browse files
kawasakiaxboe
authored andcommitted
block: change blk_mq_add_to_batch() third argument type to bool
Commit 1f47ed2 ("block: cleanup and fix batch completion adding conditions") modified the evaluation criteria for the third argument, 'ioerror', in the blk_mq_add_to_batch() function. Initially, the function had checked if 'ioerror' equals zero. Following the commit, it started checking for negative error values, with the presumption that such values, for instance -EIO, would be passed in. However, blk_mq_add_to_batch() callers do not pass negative error values. Instead, they pass status codes defined in various ways: - NVMe PCI and Apple drivers pass NVMe status code - virtio_blk driver passes the virtblk request header status byte - null_blk driver passes blk_status_t These codes are either zero or positive, therefore the revised check fails to function as intended. Specifically, with the NVMe PCI driver, this modification led to the failure of the blktests test case nvme/039. In this test scenario, errors are artificially injected to the NVMe driver, resulting in positive NVMe status codes passed to blk_mq_add_to_batch(), which unexpectedly processes the failed I/O in a batch. Hence the failure. To correct the ioerror check within blk_mq_add_to_batch(), make all callers to uniformly pass the argument as boolean. Modify the callers to check their specific status codes and pass the boolean value 'is_error'. Also describe the arguments of blK_mq_add_to_batch as kerneldoc. Fixes: 1f47ed2 ("block: cleanup and fix batch completion adding conditions") Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> Link: https://lore.kernel.org/r/20250311104359.1767728-3-shinichiro.kawasaki@wdc.com [axboe: fold in documentation update] Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent e5c2bcc commit 9bce6b5

File tree

5 files changed

+22
-11
lines changed

5 files changed

+22
-11
lines changed

drivers/block/null_blk/main.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,8 +1549,8 @@ static int null_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
15491549
cmd = blk_mq_rq_to_pdu(req);
15501550
cmd->error = null_process_cmd(cmd, req_op(req), blk_rq_pos(req),
15511551
blk_rq_sectors(req));
1552-
if (!blk_mq_add_to_batch(req, iob, (__force int) cmd->error,
1553-
blk_mq_end_request_batch))
1552+
if (!blk_mq_add_to_batch(req, iob, cmd->error != BLK_STS_OK,
1553+
blk_mq_end_request_batch))
15541554
blk_mq_end_request(req, cmd->error);
15551555
nr++;
15561556
}

drivers/block/virtio_blk.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,11 +1207,12 @@ static int virtblk_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
12071207

12081208
while ((vbr = virtqueue_get_buf(vq->vq, &len)) != NULL) {
12091209
struct request *req = blk_mq_rq_from_pdu(vbr);
1210+
u8 status = virtblk_vbr_status(vbr);
12101211

12111212
found++;
12121213
if (!blk_mq_complete_request_remote(req) &&
1213-
!blk_mq_add_to_batch(req, iob, virtblk_vbr_status(vbr),
1214-
virtblk_complete_batch))
1214+
!blk_mq_add_to_batch(req, iob, status != VIRTIO_BLK_S_OK,
1215+
virtblk_complete_batch))
12151216
virtblk_request_done(req);
12161217
}
12171218

drivers/nvme/host/apple.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,8 @@ static inline void apple_nvme_handle_cqe(struct apple_nvme_queue *q,
599599
}
600600

601601
if (!nvme_try_complete_req(req, cqe->status, cqe->result) &&
602-
!blk_mq_add_to_batch(req, iob, nvme_req(req)->status,
602+
!blk_mq_add_to_batch(req, iob,
603+
nvme_req(req)->status != NVME_SC_SUCCESS,
603604
apple_nvme_complete_batch))
604605
apple_nvme_complete_rq(req);
605606
}

drivers/nvme/host/pci.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,8 +1130,9 @@ static inline void nvme_handle_cqe(struct nvme_queue *nvmeq,
11301130

11311131
trace_nvme_sq(req, cqe->sq_head, nvmeq->sq_tail);
11321132
if (!nvme_try_complete_req(req, cqe->status, cqe->result) &&
1133-
!blk_mq_add_to_batch(req, iob, nvme_req(req)->status,
1134-
nvme_pci_complete_batch))
1133+
!blk_mq_add_to_batch(req, iob,
1134+
nvme_req(req)->status != NVME_SC_SUCCESS,
1135+
nvme_pci_complete_batch))
11351136
nvme_pci_complete_rq(req);
11361137
}
11371138

include/linux/blk-mq.h

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -852,20 +852,28 @@ static inline bool blk_mq_is_reserved_rq(struct request *rq)
852852
return rq->rq_flags & RQF_RESV;
853853
}
854854

855-
/*
855+
/**
856+
* blk_mq_add_to_batch() - add a request to the completion batch
857+
* @req: The request to add to batch
858+
* @iob: The batch to add the request
859+
* @is_error: Specify true if the request failed with an error
860+
* @complete: The completaion handler for the request
861+
*
856862
* Batched completions only work when there is no I/O error and no special
857863
* ->end_io handler.
864+
*
865+
* Return: true when the request was added to the batch, otherwise false
858866
*/
859867
static inline bool blk_mq_add_to_batch(struct request *req,
860-
struct io_comp_batch *iob, int ioerror,
868+
struct io_comp_batch *iob, bool is_error,
861869
void (*complete)(struct io_comp_batch *))
862870
{
863871
/*
864872
* Check various conditions that exclude batch processing:
865873
* 1) No batch container
866874
* 2) Has scheduler data attached
867875
* 3) Not a passthrough request and end_io set
868-
* 4) Not a passthrough request and an ioerror
876+
* 4) Not a passthrough request and failed with an error
869877
*/
870878
if (!iob)
871879
return false;
@@ -874,7 +882,7 @@ static inline bool blk_mq_add_to_batch(struct request *req,
874882
if (!blk_rq_is_passthrough(req)) {
875883
if (req->end_io)
876884
return false;
877-
if (ioerror < 0)
885+
if (is_error)
878886
return false;
879887
}
880888

0 commit comments

Comments
 (0)