[SRU][F][PATCH 2/6] blk-mq: move failure injection out of blk_mq_complete_request
Matthew Ruffell
matthew.ruffell at canonical.com
Wed Jun 22 05:17:27 UTC 2022
From: Christoph Hellwig <hch at lst.de>
BugLink: https://bugs.launchpad.net/bugs/1896350
Move the call to blk_should_fake_timeout out of blk_mq_complete_request
and into the drivers, skipping call sites that are obvious error
handlers, and remove the now superflous blk_mq_force_complete_rq helper.
This ensures we don't keep injecting errors into completions that just
terminate the Linux request after the hardware has been reset or the
command has been aborted.
Reviewed-by: Daniel Wagner <dwagner at suse.de>
Signed-off-by: Christoph Hellwig <hch at lst.de>
Signed-off-by: Jens Axboe <axboe at kernel.dk>
(backported from commit 15f73f5b3e5958f2d169fe13c420eeeeae07bbf2)
[mruffell: extensive context adjustments, no major changes]
Signed-off-by: Matthew Ruffell <matthew.ruffell at canonical.com>
---
block/blk-mq.c | 34 +++++++------------------------
block/blk-timeout.c | 6 ++----
block/blk.h | 8 --------
block/bsg-lib.c | 5 ++++-
drivers/block/loop.c | 6 ++++--
drivers/block/mtip32xx/mtip32xx.c | 3 ++-
drivers/block/nbd.c | 5 ++++-
drivers/block/null_blk_main.c | 3 ++-
drivers/block/skd_main.c | 9 +++++---
drivers/block/virtio_blk.c | 3 ++-
drivers/block/xen-blkfront.c | 3 ++-
drivers/md/dm-rq.c | 3 ++-
drivers/mmc/core/block.c | 6 +++---
drivers/nvme/host/nvme.h | 3 ++-
drivers/s390/block/dasd.c | 2 +-
drivers/s390/block/scm_blk.c | 3 ++-
drivers/scsi/scsi_lib.c | 12 +++--------
include/linux/blk-mq.h | 12 +++++++++--
18 files changed, 58 insertions(+), 68 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 82e93cd9f60d..844ca4a61247 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -580,16 +580,13 @@ static void __blk_mq_complete_request_remote(void *data)
}
/**
- * blk_mq_force_complete_rq() - Force complete the request, bypassing any error
- * injection that could drop the completion.
- * @rq: Request to be force completed
+ * blk_mq_complete_request - end I/O on a request
+ * @rq: the request being processed
*
- * Drivers should use blk_mq_complete_request() to complete requests in their
- * normal IO path. For timeout error recovery, drivers may call this forced
- * completion routine after they've reclaimed timed out requests to bypass
- * potentially subsequent fake timeouts.
- */
-void blk_mq_force_complete_rq(struct request *rq)
+ * Description:
+ * Complete a request by scheduling the ->complete_rq operation.
+ **/
+void blk_mq_complete_request(struct request *rq)
{
struct blk_mq_ctx *ctx = rq->mq_ctx;
struct request_queue *q = rq->q;
@@ -635,7 +632,7 @@ void blk_mq_force_complete_rq(struct request *rq)
}
put_cpu();
}
-EXPORT_SYMBOL_GPL(blk_mq_force_complete_rq);
+EXPORT_SYMBOL(blk_mq_complete_request);
static void hctx_unlock(struct blk_mq_hw_ctx *hctx, int srcu_idx)
__releases(hctx->srcu)
@@ -657,23 +654,6 @@ static void hctx_lock(struct blk_mq_hw_ctx *hctx, int *srcu_idx)
*srcu_idx = srcu_read_lock(hctx->srcu);
}
-/**
- * blk_mq_complete_request - end I/O on a request
- * @rq: the request being processed
- *
- * Description:
- * Ends all I/O on a request. It does not handle partial completions.
- * The actual completion happens out-of-order, through a IPI handler.
- **/
-bool blk_mq_complete_request(struct request *rq)
-{
- if (unlikely(blk_should_fake_timeout(rq->q)))
- return false;
- blk_mq_force_complete_rq(rq);
- return true;
-}
-EXPORT_SYMBOL(blk_mq_complete_request);
-
int blk_mq_request_started(struct request *rq)
{
return blk_mq_rq_state(rq) != MQ_RQ_IDLE;
diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index 8aa68fae96ad..3a1ac6434758 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -20,13 +20,11 @@ static int __init setup_fail_io_timeout(char *str)
}
__setup("fail_io_timeout=", setup_fail_io_timeout);
-int blk_should_fake_timeout(struct request_queue *q)
+bool __blk_should_fake_timeout(struct request_queue *q)
{
- if (!test_bit(QUEUE_FLAG_FAIL_IO, &q->queue_flags))
- return 0;
-
return should_fail(&fail_io_timeout, 1);
}
+EXPORT_SYMBOL_GPL(__blk_should_fake_timeout);
static int __init fail_io_timeout_debugfs(void)
{
diff --git a/block/blk.h b/block/blk.h
index ee3d5664d962..9c39d4efa4b9 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -214,17 +214,9 @@ static inline void elevator_exit(struct request_queue *q,
struct hd_struct *__disk_get_part(struct gendisk *disk, int partno);
-#ifdef CONFIG_FAIL_IO_TIMEOUT
-int blk_should_fake_timeout(struct request_queue *);
ssize_t part_timeout_show(struct device *, struct device_attribute *, char *);
ssize_t part_timeout_store(struct device *, struct device_attribute *,
const char *, size_t);
-#else
-static inline int blk_should_fake_timeout(struct request_queue *q)
-{
- return 0;
-}
-#endif
void __blk_queue_split(struct request_queue *q, struct bio **bio,
unsigned int *nr_segs);
diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 6cbb7926534c..fb7b347f8010 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -181,9 +181,12 @@ EXPORT_SYMBOL_GPL(bsg_job_get);
void bsg_job_done(struct bsg_job *job, int result,
unsigned int reply_payload_rcv_len)
{
+ struct request *rq = blk_mq_rq_from_pdu(job);
+
job->result = result;
job->reply_payload_rcv_len = reply_payload_rcv_len;
- blk_mq_complete_request(blk_mq_rq_from_pdu(job));
+ if (likely(!blk_should_fake_timeout(rq->q)))
+ blk_mq_complete_request(rq);
}
EXPORT_SYMBOL_GPL(bsg_job_done);
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 67014f84fa71..5ab7985fff8a 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -497,7 +497,8 @@ static void lo_rw_aio_do_completion(struct loop_cmd *cmd)
return;
kfree(cmd->bvec);
cmd->bvec = NULL;
- blk_mq_complete_request(rq);
+ if (likely(!blk_should_fake_timeout(rq->q)))
+ blk_mq_complete_request(rq);
}
static void lo_rw_aio_complete(struct kiocb *iocb, long ret, long ret2)
@@ -2027,7 +2028,8 @@ static void loop_handle_cmd(struct loop_cmd *cmd)
/* complete non-aio request */
if (!cmd->use_aio || ret) {
cmd->ret = ret ? -EIO : 0;
- blk_mq_complete_request(rq);
+ if (likely(!blk_should_fake_timeout(rq->q)))
+ blk_mq_complete_request(rq);
}
}
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 964f78cfffa0..2a7be513c5da 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -492,7 +492,8 @@ static void mtip_complete_command(struct mtip_cmd *cmd, blk_status_t status)
struct request *req = blk_mq_rq_from_pdu(cmd);
cmd->status = status;
- blk_mq_complete_request(req);
+ if (likely(!blk_should_fake_timeout(req->q)))
+ blk_mq_complete_request(req);
}
/*
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 9361539258af..15a8b499b738 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -773,6 +773,7 @@ static void recv_work(struct work_struct *work)
struct nbd_device *nbd = args->nbd;
struct nbd_config *config = nbd->config;
struct nbd_cmd *cmd;
+ struct request *rq;
while (1) {
cmd = nbd_read_stat(nbd, args->index);
@@ -785,7 +786,9 @@ static void recv_work(struct work_struct *work)
break;
}
- blk_mq_complete_request(blk_mq_rq_from_pdu(cmd));
+ rq = blk_mq_rq_from_pdu(cmd);
+ if (likely(!blk_should_fake_timeout(rq->q)))
+ blk_mq_complete_request(rq);
}
nbd_config_put(nbd);
atomic_dec(&config->recv_threads);
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 13eae973eaea..6938e00e08ea 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -1190,7 +1190,8 @@ static inline void nullb_complete_cmd(struct nullb_cmd *cmd)
case NULL_IRQ_SOFTIRQ:
switch (cmd->nq->dev->queue_mode) {
case NULL_Q_MQ:
- blk_mq_complete_request(cmd->rq);
+ if (likely(!blk_should_fake_timeout(cmd->rq->q)))
+ blk_mq_complete_request(cmd->rq);
break;
case NULL_Q_BIO:
/*
diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
index 51569c199a6c..3a476dc1d14f 100644
--- a/drivers/block/skd_main.c
+++ b/drivers/block/skd_main.c
@@ -1417,7 +1417,8 @@ static void skd_resolve_req_exception(struct skd_device *skdev,
case SKD_CHECK_STATUS_REPORT_GOOD:
case SKD_CHECK_STATUS_REPORT_SMART_ALERT:
skreq->status = BLK_STS_OK;
- blk_mq_complete_request(req);
+ if (likely(!blk_should_fake_timeout(req->q)))
+ blk_mq_complete_request(req);
break;
case SKD_CHECK_STATUS_BUSY_IMMINENT:
@@ -1440,7 +1441,8 @@ static void skd_resolve_req_exception(struct skd_device *skdev,
case SKD_CHECK_STATUS_REPORT_ERROR:
default:
skreq->status = BLK_STS_IOERR;
- blk_mq_complete_request(req);
+ if (likely(!blk_should_fake_timeout(req->q)))
+ blk_mq_complete_request(req);
break;
}
}
@@ -1560,7 +1562,8 @@ static int skd_isr_completion_posted(struct skd_device *skdev,
*/
if (likely(cmp_status == SAM_STAT_GOOD)) {
skreq->status = BLK_STS_OK;
- blk_mq_complete_request(rq);
+ if (likely(!blk_should_fake_timeout(rq->q)))
+ blk_mq_complete_request(rq);
} else {
skd_resolve_req_exception(skdev, skreq, rq);
}
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 2a5cd502feae..15bf7dceddf6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -272,7 +272,8 @@ static void virtblk_done(struct virtqueue *vq)
while ((vbr = virtqueue_get_buf(vblk->vqs[qid].vq, &len)) != NULL) {
struct request *req = blk_mq_rq_from_pdu(vbr);
- blk_mq_complete_request(req);
+ if (likely(!blk_should_fake_timeout(req->q)))
+ blk_mq_complete_request(req);
req_done = true;
}
if (unlikely(virtqueue_is_broken(vq)))
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 3731066f2c1c..a1d1cd25a10f 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1699,7 +1699,8 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id)
BUG();
}
- blk_mq_complete_request(req);
+ if (likely(!blk_should_fake_timeout(req->q)))
+ blk_mq_complete_request(req);
}
rinfo->ring.rsp_cons = i;
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 6bc61927d320..fb2add32d96e 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -285,7 +285,8 @@ static void dm_complete_request(struct request *rq, blk_status_t error)
struct dm_rq_target_io *tio = tio_from_request(rq);
tio->error = error;
- blk_mq_complete_request(rq);
+ if (likely(!blk_should_fake_timeout(rq->q)))
+ blk_mq_complete_request(rq);
}
/*
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 362ad361d586..4f885a050301 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1512,7 +1512,7 @@ static void mmc_blk_cqe_req_done(struct mmc_request *mrq)
*/
if (mq->in_recovery)
mmc_blk_cqe_complete_rq(mq, req);
- else
+ else if (likely(!blk_should_fake_timeout(req->q)))
blk_mq_complete_request(req);
}
@@ -1946,7 +1946,7 @@ void mmc_blk_mq_complete(struct request *req)
if (mq->use_cqe)
mmc_blk_cqe_complete_rq(mq, req);
- else
+ else if (likely(!blk_should_fake_timeout(req->q)))
mmc_blk_mq_complete_rq(mq, req);
}
@@ -1998,7 +1998,7 @@ static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req)
*/
if (mq->in_recovery)
mmc_blk_mq_complete_rq(mq, req);
- else
+ else if (likely(!blk_should_fake_timeout(req->q)))
blk_mq_complete_request(req);
mmc_blk_mq_dec_in_flight(mq, req);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 2df90d4355b9..14bfdc5d8782 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -453,7 +453,8 @@ static inline void nvme_end_request(struct request *req, __le16 status,
rq->result = result;
/* inject error when permitted by fault injection framework */
nvme_should_fail(req);
- blk_mq_complete_request(req);
+ if (likely(!blk_should_fake_timeout(req->q)))
+ blk_mq_complete_request(req);
}
static inline void nvme_get_ctrl(struct nvme_ctrl *ctrl)
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index b577c8f7e346..2adfab552e22 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2802,7 +2802,7 @@ static void __dasd_cleanup_cqr(struct dasd_ccw_req *cqr)
blk_update_request(req, BLK_STS_OK,
blk_rq_bytes(req) - proc_bytes);
blk_mq_requeue_request(req, true);
- } else {
+ } else if (likely(!blk_should_fake_timeout(req->q))) {
blk_mq_complete_request(req);
}
}
diff --git a/drivers/s390/block/scm_blk.c b/drivers/s390/block/scm_blk.c
index e01889394c84..a4f6f2e62b1d 100644
--- a/drivers/s390/block/scm_blk.c
+++ b/drivers/s390/block/scm_blk.c
@@ -256,7 +256,8 @@ static void scm_request_finish(struct scm_request *scmrq)
for (i = 0; i < nr_requests_per_io && scmrq->request[i]; i++) {
error = blk_mq_rq_to_pdu(scmrq->request[i]);
*error = scmrq->error;
- blk_mq_complete_request(scmrq->request[i]);
+ if (likely(!blk_should_fake_timeout(scmrq->request[i]->q)))
+ blk_mq_complete_request(scmrq->request[i]);
}
atomic_dec(&bdev->queued_reqs);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8e6d7ba95df1..6ebb3cf52578 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1618,18 +1618,12 @@ static blk_status_t scsi_mq_prep_fn(struct request *req)
static void scsi_mq_done(struct scsi_cmnd *cmd)
{
+ if (unlikely(blk_should_fake_timeout(cmd->request->q)))
+ return;
if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd->state)))
return;
trace_scsi_dispatch_cmd_done(cmd);
-
- /*
- * If the block layer didn't complete the request due to a timeout
- * injection, scsi must clear its internal completed state so that the
- * timeout handler will see it needs to escalate its own error
- * recovery.
- */
- if (unlikely(!blk_mq_complete_request(cmd->request)))
- clear_bit(SCMD_STATE_COMPLETE, &cmd->state);
+ blk_mq_complete_request(cmd->request);
}
static void scsi_mq_put_budget(struct blk_mq_hw_ctx *hctx)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 92b48a8e4af3..0732dec55650 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -311,8 +311,7 @@ void __blk_mq_end_request(struct request *rq, blk_status_t error);
void blk_mq_requeue_request(struct request *rq, bool kick_requeue_list);
void blk_mq_kick_requeue_list(struct request_queue *q);
void blk_mq_delay_kick_requeue_list(struct request_queue *q, unsigned long msecs);
-bool blk_mq_complete_request(struct request *rq);
-void blk_mq_force_complete_rq(struct request *rq);
+void blk_mq_complete_request(struct request *rq);
bool blk_mq_bio_list_merge(struct request_queue *q, struct list_head *list,
struct bio *bio, unsigned int nr_segs);
bool blk_mq_queue_stopped(struct request_queue *q);
@@ -344,6 +343,15 @@ void blk_mq_quiesce_queue_nowait(struct request_queue *q);
unsigned int blk_mq_rq_cpu(struct request *rq);
+bool __blk_should_fake_timeout(struct request_queue *q);
+static inline bool blk_should_fake_timeout(struct request_queue *q)
+{
+ if (IS_ENABLED(CONFIG_FAIL_IO_TIMEOUT) &&
+ test_bit(QUEUE_FLAG_FAIL_IO, &q->queue_flags))
+ return __blk_should_fake_timeout(q);
+ return false;
+}
+
/*
* Driver command data is immediately after the request. So subtract request
* size to get back to the original request, add request size to get the PDU.
--
2.34.1
More information about the kernel-team
mailing list