On Thu, Oct 31, 2019 at 6:12 AM Deepak Ukey <deepak.ukey@xxxxxxxxxxxxx> wrote:
>
> From: peter chang <dpf@xxxxxxxxxx>
>
> Added the fix so the if driver properly sent the abort it try to remove
> it from the firmware's list of outstanding commands regardless of the
> abort status. This means that the task gets free-ed 'now' rather than
> possibly getting free-ed later when the scsi layer thinks it's leaked
> but still valid.
>
> Signed-off-by: peter chang <dpf@xxxxxxxxxx>
> Signed-off-by: Deepak Ukey <deepak.ukey@xxxxxxxxxxxxx>
> Signed-off-by: Viswas G <Viswas.G@xxxxxxxxxxxxx>
> ---
> drivers/scsi/pm8001/pm8001_sas.c | 50 +++++++++++++++++++++++++++++-----------
> 1 file changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
> index 4491de8d40fc..b562916a3d3d 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.c
> +++ b/drivers/scsi/pm8001/pm8001_sas.c
> @@ -1204,8 +1204,8 @@ int pm8001_abort_task(struct sas_task *task)
> pm8001_dev = dev->lldd_dev;
> pm8001_ha = pm8001_find_ha_by_dev(dev);
> phy_id = pm8001_dev->attached_phy;
> - rc = pm8001_find_tag(task, &tag);
> - if (rc == 0) {
> + ret = pm8001_find_tag(task, &tag);
> + if (ret == 0) {
> pm8001_printk("no tag for task:%p\n", task);
> return TMF_RESP_FUNC_FAILED;
> }
> @@ -1243,26 +1243,50 @@ int pm8001_abort_task(struct sas_task *task)
>
> /* 2. Send Phy Control Hard Reset */
> reinit_completion(&completion);
> + phy->port_reset_status = PORT_RESET_TMO;
> phy->reset_success = false;
> phy->enable_completion = &completion;
> phy->reset_completion = &completion_reset;
> ret = PM8001_CHIP_DISP->phy_ctl_req(pm8001_ha, phy_id,
> PHY_HARD_RESET);
> - if (ret)
> - goto out;
> - PM8001_MSG_DBG(pm8001_ha,
> - pm8001_printk("Waiting for local phy ctl\n"));
> - wait_for_completion(&completion);
> - if (!phy->reset_success)
> + if (ret) {
> + phy->enable_completion = NULL;
> + phy->reset_completion = NULL;
> goto out;
> + }
>
> - /* 3. Wait for Port Reset complete / Port reset TMO */
> + /* In the case of the reset timeout/fail we still
> + * abort the command at the firmware. Yhe assumption
s/Yhe/The
> + * here is that the drive is off doing something so
> + * that it's not processing requests, and we want to
> + * avoid getting a completion for this and either
> + * leaking the task in libsas or losing the race and
> + * getting a double free.
> + */
> PM8001_MSG_DBG(pm8001_ha,
> + pm8001_printk("Waiting for local phy ctl\n"));
> + ret = wait_for_completion_timeout(&completion,
> + PM8001_TASK_TIMEOUT * HZ);
> + if (!ret || !phy->reset_success) {
> + phy->enable_completion = NULL;
> + phy->reset_completion = NULL;
> + } else {
> + /* 3. Wait for Port Reset complete or
> + * Port reset TMO
> + */
> + PM8001_MSG_DBG(pm8001_ha,
> pm8001_printk("Waiting for Port reset\n"));
> - wait_for_completion(&completion_reset);
> - if (phy->port_reset_status) {
> - pm8001_dev_gone_notify(dev);
> - goto out;
> + ret = wait_for_completion_timeout(
> + &completion_reset,
> + PM8001_TASK_TIMEOUT * HZ);
> + if (!ret)
> + phy->reset_completion = NULL;
> + WARN_ON(phy->port_reset_status ==
> + PORT_RESET_TMO);
> + if (phy->port_reset_status == PORT_RESET_TMO) {
> + pm8001_dev_gone_notify(dev);
> + goto out;
> + }
> }
>
> /*
> --
> 2.16.3
>
Patch looks fine, please fix the typo above.
Acked-by: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxx>
