On 2018/1/9 15:34, Hannes Reinecke wrote:
On 01/09/2018 05:09 AM, Jason Yan wrote:
Hannes,
On 2018/1/8 20:04, Hannes Reinecke wrote:
The abort handler might be racing with command completion, so the
task might already be NULL by the time the abort handler is called.
Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
---
drivers/scsi/libsas/sas_scsi_host.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/libsas/sas_scsi_host.c
b/drivers/scsi/libsas/sas_scsi_host.c
index 58476b7..08203fb 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -486,18 +486,34 @@ static int sas_queue_reset(struct domain_device
*dev, int reset_type,
int sas_eh_abort_handler(struct scsi_cmnd *cmd)
{
- int res;
- struct sas_task *task = TO_SAS_TASK(cmd);
+ int res = TMF_RESP_FUNC_COMPLETE;
+ struct sas_task *task;
struct Scsi_Host *host = cmd->device->host;
struct sas_internal *i = to_sas_internal(host->transportt);
+ struct domain_device *dev = cmd_to_domain_dev(cmd);
+ struct sas_ha_struct *ha = dev->port->ha;
+ unsigned long flags;
if (!i->dft->lldd_abort_task)
return FAILED;
- res = i->dft->lldd_abort_task(task);
+ /* Avoid sas_scsi_task_done() interfering */
+ spin_lock_irqsave(&dev->done_lock, flags);
+ task = TO_SAS_TASK(cmd);
+ if (test_bit(SAS_HA_FROZEN, &ha->state)) {
+ res = TMF_RESP_FUNC_FAILED;
+ task = NULL;
+ } else
+ ASSIGN_SAS_TASK(cmd, NULL);
+ spin_unlock_irqrestore(&dev->done_lock, flags);
+ if (task)
+ res = i->dft->lldd_abort_task(task);
if (res == TMF_RESP_FUNC_SUCC || res == TMF_RESP_FUNC_COMPLETE)
return SUCCESS;
+ spin_lock_irqsave(&dev->done_lock, flags);
+ ASSIGN_SAS_TASK(cmd, task);
+ spin_unlock_irqrestore(&dev->done_lock, flags);
Why do you assign task back? As I remember, when this cmd dispatch
again, we will create a new task and assign to it again. So should we
end this task here?
We only will create a new task if we decide to retry the command.
But if we return FAILED here the command is not retried but rather the
SCSI EH is invoked.
I got it. The SCSI EH will handle this.
But when we return SUCCESS above, shall we free the task? I don't see
that LLDDs free it.
Jason
Cheers,
Hannes