[Unstable,Artful,SRU,Zesty] scsi: hisi_sas: fix timeout check in hisi_sas_internal_task_abort()

Submitted by dann frazier on Aug. 4, 2017, 7:19 p.m.

Details

Message ID 20170804191928.4iwgp6bqtimgui6f@xps13.dannf
State New
Headers show

Commit Message

dann frazier Aug. 4, 2017, 7:19 p.m.
From: Xiang Chen <chenxiang66@hisilicon.com>

BugLink: https://bugs.launchpad.net/bugs/1708730

We need to check for timeout before task status, or the task will be
mistook as completed internal abort command.  Also add protection for
sas_task.task_state_flags in hisi_sas_tmf_timedout().

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
(cherry picked from commit f64a6988268aae866bb6ce6edb910d454ccef331)
Signed-off-by: dann frazier <dann.frazier@canonical.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

Comments

Seth Forshee Aug. 7, 2017, 2:01 p.m.
On Fri, Aug 04, 2017 at 01:19:28PM -0600, dann frazier wrote:
> From: Xiang Chen <chenxiang66@hisilicon.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1708730
> 
> We need to check for timeout before task status, or the task will be
> mistook as completed internal abort command.  Also add protection for
> sas_task.task_state_flags in hisi_sas_tmf_timedout().
> 
> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> Signed-off-by: John Garry <john.garry@huawei.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> (cherry picked from commit f64a6988268aae866bb6ce6edb910d454ccef331)
> Signed-off-by: dann frazier <dann.frazier@canonical.com>

Clean cherry-pick, scope limited to a single platform-specific driver.

Acked-by: Seth Forshee <seth.forshee@canonical.com>

Applied to artful/master-next and unstable/master, thanks.
Stefan Bader Aug. 8, 2017, 9:49 a.m.
On 04.08.2017 21:19, dann frazier wrote:
> From: Xiang Chen <chenxiang66@hisilicon.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1708730
> 
> We need to check for timeout before task status, or the task will be
> mistook as completed internal abort command.  Also add protection for
> sas_task.task_state_flags in hisi_sas_tmf_timedout().
> 
> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> Signed-off-by: John Garry <john.garry@huawei.com>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> (cherry picked from commit f64a6988268aae866bb6ce6edb910d454ccef331)
> Signed-off-by: dann frazier <dann.frazier@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index 938a22f12868..60592a4b65a2 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -691,8 +691,13 @@ static void hisi_sas_task_done(struct sas_task *task)
>  static void hisi_sas_tmf_timedout(unsigned long data)
>  {
>  	struct sas_task *task = (struct sas_task *)data;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&task->task_state_lock, flags);
> +	if (!(task->task_state_flags & SAS_TASK_STATE_DONE))
> +		task->task_state_flags |= SAS_TASK_STATE_ABORTED;
> +	spin_unlock_irqrestore(&task->task_state_lock, flags);
>  
> -	task->task_state_flags |= SAS_TASK_STATE_ABORTED;
>  	complete(&task->slow_task->completion);
>  }
>  
> @@ -1247,6 +1252,17 @@ hisi_sas_internal_task_abort(struct hisi_hba *hisi_hba,
>  	wait_for_completion(&task->slow_task->completion);
>  	res = TMF_RESP_FUNC_FAILED;
>  
> +	/* Internal abort timed out */
> +	if ((task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
> +		if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
> +			struct hisi_sas_slot *slot = task->lldd_task;
> +
> +			if (slot)
> +				slot->task = NULL;
> +			dev_err(dev, "internal task abort: timeout.\n");
> +		}
> +	}
> +
>  	if (task->task_status.resp == SAS_TASK_COMPLETE &&
>  		task->task_status.stat == TMF_RESP_FUNC_COMPLETE) {
>  		res = TMF_RESP_FUNC_COMPLETE;
> @@ -1259,13 +1275,6 @@ hisi_sas_internal_task_abort(struct hisi_hba *hisi_hba,
>  		goto exit;
>  	}
>  
> -	/* Internal abort timed out */
> -	if ((task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
> -		if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
> -			dev_err(dev, "internal task abort: timeout.\n");
> -		}
> -	}
> -
>  exit:
>  	dev_dbg(dev, "internal task abort: task to dev %016llx task=%p "
>  		"resp: 0x%x sts 0x%x\n",
>
Kleber Souza Aug. 8, 2017, 9:52 a.m.
Applied on zesty/master-next branch. Thanks.

Patch hide | download patch | download mbox

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 938a22f12868..60592a4b65a2 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -691,8 +691,13 @@  static void hisi_sas_task_done(struct sas_task *task)
 static void hisi_sas_tmf_timedout(unsigned long data)
 {
 	struct sas_task *task = (struct sas_task *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&task->task_state_lock, flags);
+	if (!(task->task_state_flags & SAS_TASK_STATE_DONE))
+		task->task_state_flags |= SAS_TASK_STATE_ABORTED;
+	spin_unlock_irqrestore(&task->task_state_lock, flags);
 
-	task->task_state_flags |= SAS_TASK_STATE_ABORTED;
 	complete(&task->slow_task->completion);
 }
 
@@ -1247,6 +1252,17 @@  hisi_sas_internal_task_abort(struct hisi_hba *hisi_hba,
 	wait_for_completion(&task->slow_task->completion);
 	res = TMF_RESP_FUNC_FAILED;
 
+	/* Internal abort timed out */
+	if ((task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
+		if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
+			struct hisi_sas_slot *slot = task->lldd_task;
+
+			if (slot)
+				slot->task = NULL;
+			dev_err(dev, "internal task abort: timeout.\n");
+		}
+	}
+
 	if (task->task_status.resp == SAS_TASK_COMPLETE &&
 		task->task_status.stat == TMF_RESP_FUNC_COMPLETE) {
 		res = TMF_RESP_FUNC_COMPLETE;
@@ -1259,13 +1275,6 @@  hisi_sas_internal_task_abort(struct hisi_hba *hisi_hba,
 		goto exit;
 	}
 
-	/* Internal abort timed out */
-	if ((task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
-		if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
-			dev_err(dev, "internal task abort: timeout.\n");
-		}
-	}
-
 exit:
 	dev_dbg(dev, "internal task abort: task to dev %016llx task=%p "
 		"resp: 0x%x sts 0x%x\n",