Commit f4829a9b authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Jens Axboe
Browse files

blk-mq: fix racy updates of rq->errors 



blk_mq_complete_request may be a no-op if the request has already
been completed by others means (e.g. a timeout or cancellation), but
currently drivers have to set rq->errors before calling
blk_mq_complete_request, which might leave us with the wrong error value.

Add an error parameter to blk_mq_complete_request so that we can
defer setting rq->errors until we known we won the race to complete the
request.

Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarSagi Grimberg <sagig@mellanox.com>
Signed-off-by: default avatarJens Axboe <axboe@fb.com>
parent 60de074b

block/blk-mq.c

+6 −6
Original line number Diff line number Diff line
@@ -393,15 +393,17 @@ void __blk_mq_complete_request(struct request *rq) 
 *	Ends all I/O on a request. It does not handle partial completions.

 *	The actual completion happens out-of-order, through a IPI handler.

 **/

void blk_mq_complete_request(struct request *rq)

void blk_mq_complete_request(struct request *rq, int error)

{

	struct request_queue *q = rq->q;



	if (unlikely(blk_should_fake_timeout(q)))

		return;

	if (!blk_mark_rq_complete(rq))

	if (!blk_mark_rq_complete(rq)) {

		rq->errors = error;

		__blk_mq_complete_request(rq);

	}

}

EXPORT_SYMBOL(blk_mq_complete_request);



int blk_mq_request_started(struct request *rq)
@@ -616,10 +618,8 @@ static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx, 
		 * If a request wasn't started before the queue was

		 * marked dying, kill it here or it'll go unnoticed.

		 */

		if (unlikely(blk_queue_dying(rq->q))) {

			rq->errors = -EIO;

			blk_mq_complete_request(rq);

		}

		if (unlikely(blk_queue_dying(rq->q)))

			blk_mq_complete_request(rq, -EIO);

		return;

	}

	if (rq->cmd_flags & REQ_NO_TIMEOUT)

drivers/block/loop.c

+5 −6
Original line number Diff line number Diff line
@@ -1486,17 +1486,16 @@ static void loop_handle_cmd(struct loop_cmd *cmd) 
{

	const bool write = cmd->rq->cmd_flags & REQ_WRITE;

	struct loop_device *lo = cmd->rq->q->queuedata;

	int ret = -EIO;

	int ret = 0;



	if (write && (lo->lo_flags & LO_FLAGS_READ_ONLY))

	if (write && (lo->lo_flags & LO_FLAGS_READ_ONLY)) {

		ret = -EIO;

		goto failed;

	}



	ret = do_req_filebacked(lo, cmd->rq);



 failed:

	if (ret)

		cmd->rq->errors = -EIO;

	blk_mq_complete_request(cmd->rq);

	blk_mq_complete_request(cmd->rq, ret ? -EIO : 0);

}



static void loop_queue_write_work(struct work_struct *work)

drivers/block/null_blk.c

+1 −1
Original line number Diff line number Diff line
@@ -289,7 +289,7 @@ static inline void null_handle_cmd(struct nullb_cmd *cmd) 
	case NULL_IRQ_SOFTIRQ:

		switch (queue_mode)  {

		case NULL_Q_MQ:

			blk_mq_complete_request(cmd->rq);

			blk_mq_complete_request(cmd->rq, cmd->rq->errors);

			break;

		case NULL_Q_RQ:

			blk_complete_request(cmd->rq);

drivers/block/nvme-core.c

+7 −9
Original line number Diff line number Diff line
@@ -618,16 +618,15 @@ static void req_completion(struct nvme_queue *nvmeq, void *ctx, 
			spin_unlock_irqrestore(req->q->queue_lock, flags);

			return;

		}



		if (req->cmd_type == REQ_TYPE_DRV_PRIV) {

			if (cmd_rq->ctx == CMD_CTX_CANCELLED)

				req->errors = -EINTR;

			else

				req->errors = status;

				status = -EINTR;

		} else {

			req->errors = nvme_error_status(status);

			status = nvme_error_status(status);

		}

	} else

		req->errors = 0;

	}



	if (req->cmd_type == REQ_TYPE_DRV_PRIV) {

		u32 result = le32_to_cpup(&cqe->result);

		req->special = (void *)(uintptr_t)result;
@@ -650,7 +649,7 @@ static void req_completion(struct nvme_queue *nvmeq, void *ctx, 
	}

	nvme_free_iod(nvmeq->dev, iod);



	blk_mq_complete_request(req);

	blk_mq_complete_request(req, status);

}



/* length is in bytes.  gfp flags indicates whether we may sleep. */
@@ -863,8 +862,7 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx, 
	if (ns && ns->ms && !blk_integrity_rq(req)) {

		if (!(ns->pi_type && ns->ms == 8) &&

					req->cmd_type != REQ_TYPE_DRV_PRIV) {

			req->errors = -EFAULT;

			blk_mq_complete_request(req);

			blk_mq_complete_request(req, -EFAULT);

			return BLK_MQ_RQ_QUEUE_OK;

		}

	}

drivers/block/virtio_blk.c

+1 −1
Original line number Diff line number Diff line
@@ -144,7 +144,7 @@ static void virtblk_done(struct virtqueue *vq) 
	do {

		virtqueue_disable_cb(vq);

		while ((vbr = virtqueue_get_buf(vblk->vqs[qid].vq, &len)) != NULL) {

			blk_mq_complete_request(vbr->req);

			blk_mq_complete_request(vbr->req, vbr->req->errors);

			req_done = true;

		}

		if (unlikely(virtqueue_is_broken(vq)))

CRA Git | Maintained and supported by SUSTech CRA and CCSE