blk-mq: move failure injection out of blk_mq_complete_request
authorChristoph Hellwig <hch@lst.de>
Thu, 11 Jun 2020 06:44:47 +0000 (08:44 +0200)
committerJens Axboe <axboe@kernel.dk>
Wed, 24 Jun 2020 15:15:57 +0000 (09:15 -0600)
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@suse.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
19 files changed:
block/blk-mq.c
block/blk-timeout.c
block/blk.h
block/bsg-lib.c
drivers/block/loop.c
drivers/block/mtip32xx/mtip32xx.c
drivers/block/nbd.c
drivers/block/null_blk_main.c
drivers/block/skd_main.c
drivers/block/virtio_blk.c
drivers/block/xen-blkfront.c
drivers/md/dm-rq.c
drivers/mmc/core/block.c
drivers/nvme/host/core.c
drivers/nvme/host/nvme.h
drivers/s390/block/dasd.c
drivers/s390/block/scm_blk.c
drivers/scsi/scsi_lib.c
include/linux/blk-mq.h

index ce772ab191888ea2446e14ae094834ceb0b8acc3..3f4f227cf830c7ba4a2fbb49c2d2ac80a7f5dab5 100644 (file)
@@ -655,16 +655,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;
@@ -702,7 +699,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)
@@ -724,23 +721,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);
-
 /**
  * blk_mq_start_request - Start processing a request
  * @rq: Pointer to request to be started
index 8aa68fae96ad8d87b41890cc8a77654a60feecbc..3a1ac64347588d564df6e6d51d9ce4865f4853c4 100644 (file)
@@ -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)
 {
index b5d1f0fc6547c746785d6f651d6b649ae80e4be2..8ba4a5e4fe07ac006afe2c43c893214e6b28978a 100644 (file)
@@ -223,18 +223,9 @@ ssize_t part_fail_show(struct device *dev, struct device_attribute *attr,
                char *buf);
 ssize_t part_fail_store(struct device *dev, struct device_attribute *attr,
                const char *buf, size_t count);
-
-#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);
index 6cbb7926534cd7985e22ce280c4b29c66a900826..fb7b347f80105b142c3577f0c0bde4fc72583c6a 100644 (file)
@@ -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);
 
index 475e1a738560daa8b955955d228dccee990593fc..4acae248790ccf14fdadca7f73db2f4c6686b3aa 100644 (file)
@@ -509,7 +509,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)
@@ -2048,7 +2049,8 @@ static void loop_handle_cmd(struct loop_cmd *cmd)
                        cmd->ret = ret;
                else
                        cmd->ret = ret ? -EIO : 0;
-               blk_mq_complete_request(rq);
+               if (likely(!blk_should_fake_timeout(rq->q)))
+                       blk_mq_complete_request(rq);
        }
 }
 
index f6bafa9a68b9dfaed33af2f8940bf38536885f05..153e2cdecb4d40a43c107187b36d57da56192ae6 100644 (file)
@@ -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);
 }
 
 /*
index 43cff01a5a675d47cd9bcb60ffcf1440ac8fc871..01794cd2b6cae4f72628bd8bbd12043c3febff82 100644 (file)
@@ -784,6 +784,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);
@@ -796,7 +797,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);
        }
        atomic_dec(&config->recv_threads);
        wake_up(&config->recv_wq);
index 87b31f9ca362ee17b6407811b223842cf28df88f..82259242b9b5c96f7f8f20376187dd3c8e3436bc 100644 (file)
@@ -1283,7 +1283,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:
                        /*
@@ -1423,7 +1424,7 @@ static bool should_requeue_request(struct request *rq)
 static enum blk_eh_timer_return null_timeout_rq(struct request *rq, bool res)
 {
        pr_info("rq %p timed out\n", rq);
-       blk_mq_force_complete_rq(rq);
+       blk_mq_complete_request(rq);
        return BLK_EH_DONE;
 }
 
index 51569c199a6cc998356cbbba7e67c6c1ecfdf8ad..3a476dc1d14f57b5fceee13550bf9f6dacbf5590 100644 (file)
@@ -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);
                }
index 9d21bf0f155eed65fa150d71e6b92104fd701960..741804bd8a14030640f6650816041138ed65818c 100644 (file)
@@ -171,7 +171,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)))
index 3b889ea950c21c91a4d593de7530037d440c4734..3bb3dd8da9b0c101ecedef262966e5eb56f51549 100644 (file)
@@ -1655,7 +1655,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;
index f60c025121215bf6fa39f31c32858e34764dc1dd..5aec1cd093481d40eb2181e14196700d75da8371 100644 (file)
@@ -288,7 +288,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);
 }
 
 /*
index 7896952de1ac75a5bc384eb065868732b64c02e3..4791c82f8f7c78768b047199158d987e9aff9169 100644 (file)
@@ -1446,7 +1446,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);
 }
 
@@ -1926,7 +1926,7 @@ static void mmc_blk_hsq_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);
 }
 
@@ -1936,7 +1936,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);
 }
 
@@ -1988,7 +1988,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);
index c2c5bc4fb702d37590f55a933baac8ffde49aebc..6810c8812aed269d691172d4841604e4376eecd9 100644 (file)
@@ -304,7 +304,7 @@ bool nvme_cancel_request(struct request *req, void *data, bool reserved)
                return true;
 
        nvme_req(req)->status = NVME_SC_HOST_ABORTED_CMD;
-       blk_mq_force_complete_rq(req);
+       blk_mq_complete_request(req);
        return true;
 }
 EXPORT_SYMBOL_GPL(nvme_cancel_request);
index c0f4226d3299259b1e6c29f51a93a763391d3593..034613205701c9a0491052e3752e063b823d6f89 100644 (file)
@@ -481,7 +481,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)
index cf87eb27879f087cdc125e2c248916f0909a41c1..eb17fea8075c6f49d47d42e5b692b928f1adeb93 100644 (file)
@@ -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);
                }
        }
index e01889394c8412654eab27e8386f00b28e2383a8..a4f6f2e62b1dcfc03bfe247d6e7316ebe1b9b2ec 100644 (file)
@@ -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);
index 0ba7a65e7c8d96619ec0f285e0376776c281d374..6ca91d09eca1eca728cb081d953738d61b05042f 100644 (file)
@@ -1589,18 +1589,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)
index d6fcae17da5a2acd49cacdfb8c5385bde0b0981d..8e6ab766aef76e2fee85fe71d8e399b9823781eb 100644 (file)
@@ -503,8 +503,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);
@@ -537,6 +536,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;
+}
+
 /**
  * blk_mq_rq_from_pdu - cast a PDU to a request
  * @pdu: the PDU (Protocol Data Unit) to be casted