Commit 980d3aeb authored by Dan Williams's avatar Dan Williams
Browse files

isci: fix isci_terminate_pending() list management 



Walk through the list of pending requests being careful to consider that
multiple requests can be terminated when the lock is dropped (i.e.
invalidating the 'next' reference established by
list_for_each_entry_safe).

Also noticed that all callers to isci_terminate_pending_requests()
specifying terminating, so just drop the parameter.

Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
parent 77c852f3

drivers/scsi/isci/remote_device.c

+1 −1
Original line number Diff line number Diff line
@@ -1272,7 +1272,7 @@ void isci_remote_device_nuke_requests(struct isci_host *ihost, struct isci_remot 
		"%s: idev = %p\n", __func__, idev);



	/* Cleanup all requests pending for this device. */

	isci_terminate_pending_requests(ihost, idev, terminating);

	isci_terminate_pending_requests(ihost, idev);



	dev_dbg(&ihost->pdev->dev,

		"%s: idev = %p, done\n", __func__, idev);

drivers/scsi/isci/request.h

+2 −3
Original line number Diff line number Diff line
@@ -776,9 +776,8 @@ isci_request_io_request_get_next_sge(struct isci_request *request, 
}



void

isci_terminate_pending_requests(struct isci_host *isci_host,

				struct isci_remote_device *isci_device,

				enum isci_request_status new_request_state);

isci_terminate_pending_requests(struct isci_host *ihost,

				struct isci_remote_device *idev);

enum sci_status

scic_task_request_construct(struct scic_sds_controller *scic,

			    struct scic_sds_remote_device *sci_dev,

drivers/scsi/isci/task.c

+64 −67
Original line number Diff line number Diff line
@@ -796,53 +796,53 @@ static void isci_terminate_request_core( 
 * @isci_host: This parameter specifies SCU.

 * @isci_device: This parameter specifies the target.

 *

 *

 */

void isci_terminate_pending_requests(

	struct isci_host *isci_host,

	struct isci_remote_device *isci_device,

	enum isci_request_status new_request_state)

void isci_terminate_pending_requests(struct isci_host *ihost,

				     struct isci_remote_device *idev)

{

	struct isci_request *request;

	struct isci_request *next_request;

	unsigned long       flags;

	struct completion request_completion;

	enum isci_request_status old_state;

	DECLARE_COMPLETION_ONSTACK(request_completion);



	dev_dbg(&isci_host->pdev->dev,

		"%s: isci_device = %p (new request state = %d)\n",

		__func__, isci_device, new_request_state);



	spin_lock_irqsave(&isci_host->scic_lock, flags);

	unsigned long flags;

	LIST_HEAD(list);



	/* Iterate through the list. */

	list_for_each_entry_safe(request, next_request,

				 &isci_device->reqs_in_process, dev_node) {

	spin_lock_irqsave(&ihost->scic_lock, flags);

	list_splice_init(&idev->reqs_in_process, &list);



		init_completion(&request_completion);

	/* assumes that isci_terminate_request_core deletes from the list */

	while (!list_empty(&list)) {

		struct isci_request *ireq = list_entry(list.next, typeof(*ireq), dev_node);



		/* Change state to "new_request_state" if it is currently

		/* Change state to "terminating" if it is currently

		 * "started".

		 */

		old_state = isci_request_change_started_to_newstate(

					request,

		old_state = isci_request_change_started_to_newstate(ireq,

								    &request_completion,

					new_request_state);

								    terminating);

		switch (old_state) {

		case started:

		case completed:

		case aborting:

			break;

		default:

			/* termination in progress, or otherwise dispositioned.

			 * We know the request was on 'list' so should be safe

			 * to move it back to reqs_in_process

			 */

			list_move(&ireq->dev_node, &idev->reqs_in_process);

			ireq = NULL;

			break;

		}



		spin_unlock_irqrestore(&isci_host->scic_lock, flags);

		if (!ireq)

			continue;

		spin_unlock_irqrestore(&ihost->scic_lock, flags);



		if ((old_state == started) ||

		    (old_state == completed) ||

		    (old_state == aborting)) {

		init_completion(&request_completion);



			dev_warn(&isci_host->pdev->dev,

				 "%s: isci_device=%p request=%p; task=%p "

				 "old_state=%d\n",

				 __func__,

				 isci_device, request,

				 ((request->ttype == io_task)

					? isci_request_access_task(request)

					: NULL),

		dev_dbg(&ihost->pdev->dev,

			 "%s: idev=%p request=%p; task=%p old_state=%d\n",

			 __func__, idev, ireq,

			ireq->ttype == io_task ? isci_request_access_task(ireq) : NULL,

			old_state);



		/* If the old_state is started:
@@ -865,12 +865,10 @@ void isci_terminate_pending_requests( 
		 * This request has already gone through a TMF timeout, but may

		 * not have been terminated; needs cleaning up at least.

		 */

			isci_terminate_request_core(isci_host, isci_device,

						    request);

		}

		spin_lock_irqsave(&isci_host->scic_lock, flags);

		isci_terminate_request_core(ihost, idev, ireq);

		spin_lock_irqsave(&ihost->scic_lock, flags);

	}

	spin_unlock_irqrestore(&isci_host->scic_lock, flags);

	spin_unlock_irqrestore(&ihost->scic_lock, flags);

}



/**
@@ -965,8 +963,7 @@ int isci_task_lu_reset(struct domain_device *domain_device, u8 *lun) 
	if (ret == TMF_RESP_FUNC_COMPLETE)

		/* Terminate all I/O now. */

		isci_terminate_pending_requests(isci_host,

						isci_device,

						terminating);

						isci_device);



	return ret;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE