From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Garry Subject: Re: [PATCH 2/6] hisi_sas: add hisi_sas_slot_abort() Date: Tue, 16 Feb 2016 15:41:54 +0000 Message-ID: <56C34342.9050605@huawei.com> References: <1455625351-165881-1-git-send-email-john.garry@huawei.com> <1455625351-165881-3-git-send-email-john.garry@huawei.com> <56C33EC2.3020005@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from szxga02-in.huawei.com ([119.145.14.65]:27536 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932638AbcBPPnF (ORCPT ); Tue, 16 Feb 2016 10:43:05 -0500 In-Reply-To: <56C33EC2.3020005@suse.de> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Hannes Reinecke , JBottomley@odin.com, martin.petersen@oracle.com Cc: linuxarm@huawei.com, zhangfei.gao@linaro.org, xuwei5@hisilicon.com, john.garry2@mail.dcu.ie, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org On 16/02/2016 15:22, Hannes Reinecke wrote: > On 02/16/2016 01:22 PM, John Garry wrote: >> Add a function to abort a slot (task) in the device >> (if it is in the task set) and then cleanup and >> complete the task. >> The function is called from work queue context as >> it cannot be called from the context where it is >> triggered (interrupt). >> >> Signed-off-by: John Garry >> --- >> drivers/scsi/hisi_sas/hisi_sas.h | 1 + >> drivers/scsi/hisi_sas/hisi_sas_main.c | 43 +++++++++++++++++++++++++++++++++++ >> 2 files changed, 44 insertions(+) >> >> diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h >> index 02da7e4..a05ce71 100644 >> --- a/drivers/scsi/hisi_sas/hisi_sas.h >> +++ b/drivers/scsi/hisi_sas/hisi_sas.h >> @@ -120,6 +120,7 @@ struct hisi_sas_slot { >> dma_addr_t command_table_dma; >> struct hisi_sas_sge_page *sge_page; >> dma_addr_t sge_page_dma; >> + struct work_struct abort_slot; >> }; >> >> struct hisi_sas_tmf_task { >> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c >> index c600f5e..65509eb 100644 >> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c >> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c >> @@ -15,6 +15,9 @@ >> #define DEV_IS_GONE(dev) \ >> ((!dev) || (dev->dev_type == SAS_PHY_UNUSED)) >> >> +static int hisi_sas_debug_issue_ssp_tmf(struct domain_device *device, >> + u8 *lun, struct hisi_sas_tmf_task *tmf); >> + >> static struct hisi_hba *dev_to_hisi_hba(struct domain_device *device) >> { >> return device->port->ha->lldd_ha; >> @@ -113,6 +116,45 @@ static int hisi_sas_task_prep_ata(struct hisi_hba *hisi_hba, >> return hisi_hba->hw->prep_stp(hisi_hba, slot); >> } >> >> +/* >> + * This function will issue an abort TMF if a task is still in >> + * the target. Then it will do the task complete cleanup and >> + * callbacks. >> + */ >> +static void hisi_sas_slot_abort(struct work_struct *work) >> +{ >> + struct hisi_sas_slot *abort_slot = >> + container_of(work, struct hisi_sas_slot, abort_slot); >> + struct sas_task *task = abort_slot->task; >> + struct hisi_hba *hisi_hba = dev_to_hisi_hba(task->dev); >> + struct scsi_cmnd *cmnd = task->uldd_task; >> + struct hisi_sas_tmf_task tmf_task; >> + struct domain_device *device = task->dev; >> + struct hisi_sas_device *sas_dev = device->lldd_dev; >> + struct scsi_lun lun; >> + int tag = abort_slot->idx, rc; >> + >> + int_to_scsilun(cmnd->device->lun, &lun); >> + tmf_task.tmf = TMF_QUERY_TASK; >> + tmf_task.tag_of_task_to_be_managed = cpu_to_le16(tag); >> + >> + rc = hisi_sas_debug_issue_ssp_tmf(task->dev, lun.scsi_lun, &tmf_task); >> + >> + /* TMF_RESP_FUNC_SUCC means task is present in the task set */ >> + if (rc != TMF_RESP_FUNC_SUCC) >> + goto out; >> + tmf_task.tmf = TMF_ABORT_TASK; >> + tmf_task.tag_of_task_to_be_managed = cpu_to_le16(tag); >> + >> + rc = hisi_sas_debug_issue_ssp_tmf(task->dev, lun.scsi_lun, &tmf_task); >> +out: >> + hisi_sas_slot_task_free(hisi_hba, task, abort_slot); >> + if (task->task_done) >> + task->task_done(task); >> + if (sas_dev && sas_dev->running_req) >> + sas_dev->running_req--; >> +} >> + >> static int hisi_sas_task_prep(struct sas_task *task, struct hisi_hba *hisi_hba, >> int is_tmf, struct hisi_sas_tmf_task *tmf, >> int *pass) > Do you really need to query the task first? > As per SAM a successful return from an ABORT TASK TMF has this meaning: > > A response of FUNCTION COMPLETE shall indicate that the task was > aborted or was not in the task set. > > Ie it doesn't matter if the task was present or not. > > Cheers, > > Hannes > I think that it should be ok to the abort without first querying. I'll just test this to double-check. (This would make the first patch in the series superflous) Cheers, John