Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f1342709 authored by Keith Busch's avatar Keith Busch Committed by Jens Axboe
Browse files

scsi: Do not rely on blk-mq for double completions 



The scsi timeout error handling had been directly updating the block
layer's request state to prevent a error handling and a natural completion
from completing the same request twice. Fix this layering violation
by having scsi control the fate of its commands with scsi owned flags
rather than use blk-mq's.

Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarKeith Busch <keith.busch@intel.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 16c15eb1

drivers/scsi/scsi_error.c

+11 −11
Original line number Diff line number Diff line
@@ -297,19 +297,19 @@ enum blk_eh_timer_return scsi_times_out(struct request *req) 


	if (rtn == BLK_EH_DONE) {

		/*

		 * For blk-mq, we must set the request state to complete now

		 * before sending the request to the scsi error handler. This

		 * will prevent a use-after-free in the event the LLD manages

		 * to complete the request before the error handler finishes

		 * processing this timed out request.

		 * Set the command to complete first in order to prevent a real

		 * completion from releasing the command while error handling

		 * is using it. If the command was already completed, then the

		 * lower level driver beat the timeout handler, and it is safe

		 * to return without escalating error recovery.

		 *

		 * If the request was already completed, then the LLD beat the

		 * time out handler from transferring the request to the scsi

		 * error handler. In that case we can return immediately as no

		 * further action is required.

		 * If timeout handling lost the race to a real completion, the

		 * block layer may ignore that due to a fake timeout injection,

		 * so return RESET_TIMER to allow error handling another shot

		 * at this command.

		 */

		if (!blk_mq_mark_complete(req))

			return rtn;

		if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state))

			return BLK_EH_RESET_TIMER;

		if (scsi_abort_command(scmd) != SUCCESS) {

			set_host_byte(scmd, DID_TIME_OUT);

			scsi_eh_scmd_add(scmd);

drivers/scsi/scsi_lib.c

+12 −1
Original line number Diff line number Diff line
@@ -1642,8 +1642,18 @@ static blk_status_t scsi_mq_prep_fn(struct request *req) 


static void scsi_mq_done(struct scsi_cmnd *cmd)

{

	if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd->state)))

		return;

	trace_scsi_dispatch_cmd_done(cmd);

	blk_mq_complete_request(cmd->request);



	/*

	 * 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);

}



static void scsi_mq_put_budget(struct blk_mq_hw_ctx *hctx)
@@ -1702,6 +1712,7 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx, 
	if (!scsi_host_queue_ready(q, shost, sdev))

		goto out_dec_target_busy;



	clear_bit(SCMD_STATE_COMPLETE, &cmd->state);

	if (!(req->rq_flags & RQF_DONTPREP)) {

		ret = scsi_mq_prep_fn(req);

		if (ret != BLK_STS_OK)

include/scsi/scsi_cmnd.h

+4 −0
Original line number Diff line number Diff line
@@ -61,6 +61,9 @@ struct scsi_pointer { 
/* flags preserved across unprep / reprep */

#define SCMD_PRESERVED_FLAGS	(SCMD_UNCHECKED_ISA_DMA | SCMD_INITIALIZED)



/* for scmd->state */

#define SCMD_STATE_COMPLETE	(1 << 0)



struct scsi_cmnd {

	struct scsi_request req;

	struct scsi_device *device;
@@ -145,6 +148,7 @@ struct scsi_cmnd { 


	int result;		/* Status code from lower level driver */

	int flags;		/* Command flags */

	unsigned long state;	/* Command completion state */



	unsigned char tag;	/* SCSI-II queued command tag */

};