Skip to content

Commit 7612ae9

Browse files
kawasakigregkh
authored andcommitted
block: change blk_mq_add_to_batch() third argument type to bool
[ Upstream commit 9bce6b5 ] 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> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 115d1f7 commit 7612ae9

5 files changed

Lines changed: 22 additions & 11 deletions

File tree

drivers/block/null_blk/main.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1541,8 +1541,8 @@ static int null_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
15411541
cmd = blk_mq_rq_to_pdu(req);
15421542
cmd->error = null_process_cmd(cmd, req_op(req), blk_rq_pos(req),
15431543
blk_rq_sectors(req));
1544-
if (!blk_mq_add_to_batch(req, iob, (__force int) cmd->error,
1545-
blk_mq_end_request_batch))
1544+
if (!blk_mq_add_to_batch(req, iob, cmd->error != BLK_STS_OK,
1545+
blk_mq_end_request_batch))
15461546
blk_mq_end_request(req, cmd->error);
15471547
nr++;
15481548
}

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
@@ -1131,8 +1131,9 @@ static inline void nvme_handle_cqe(struct nvme_queue *nvmeq,
11311131

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

include/linux/blk-mq.h

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

866-
/*
866+
/**
867+
* blk_mq_add_to_batch() - add a request to the completion batch
868+
* @req: The request to add to batch
869+
* @iob: The batch to add the request
870+
* @is_error: Specify true if the request failed with an error
871+
* @complete: The completaion handler for the request
872+
*
867873
* Batched completions only work when there is no I/O error and no special
868874
* ->end_io handler.
875+
*
876+
* Return: true when the request was added to the batch, otherwise false
869877
*/
870878
static inline bool blk_mq_add_to_batch(struct request *req,
871-
struct io_comp_batch *iob, int ioerror,
879+
struct io_comp_batch *iob, bool is_error,
872880
void (*complete)(struct io_comp_batch *))
873881
{
874882
/*
875883
* Check various conditions that exclude batch processing:
876884
* 1) No batch container
877885
* 2) Has scheduler data attached
878886
* 3) Not a passthrough request and end_io set
879-
* 4) Not a passthrough request and an ioerror
887+
* 4) Not a passthrough request and failed with an error
880888
*/
881889
if (!iob)
882890
return false;
@@ -885,7 +893,7 @@ static inline bool blk_mq_add_to_batch(struct request *req,
885893
if (!blk_rq_is_passthrough(req)) {
886894
if (req->end_io)
887895
return false;
888-
if (ioerror < 0)
896+
if (is_error)
889897
return false;
890898
}
891899

0 commit comments

Comments
 (0)