On 01/09/2018 10:04 AM, Jason Yan wrote:
>
> 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.
>
Hmm. Possibly.
I'll check.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
