Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1335305 > unrolled thread

[PATCH 0/6] hisi_sas: add abort and retry feature

Started byJohn Garry <john.garry@huawei.com>
First post2016-02-16 13:10 +0100
Last post2016-02-16 18:00 +0100
Articles 6 on this page of 26 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] hisi_sas: add abort and retry feature John Garry <john.garry@huawei.com> - 2016-02-16 13:10 +0100
    [PATCH 1/6] hisi_sas: add TMF_RESP_FUNC_SUCC check John Garry <john.garry@huawei.com> - 2016-02-16 13:10 +0100
      Re: [PATCH 1/6] hisi_sas: add TMF_RESP_FUNC_SUCC check Hannes Reinecke <hare@suse.de> - 2016-02-16 16:30 +0100
    [PATCH 6/6] hisi_sas: update driver version to 1.3 John Garry <john.garry@huawei.com> - 2016-02-16 13:10 +0100
    [PATCH 2/6] hisi_sas: add hisi_sas_slot_abort() John Garry <john.garry@huawei.com> - 2016-02-16 13:10 +0100
      Re: [PATCH 2/6] hisi_sas: add hisi_sas_slot_abort() Hannes Reinecke <hare@suse.de> - 2016-02-16 16:30 +0100
        Re: [PATCH 2/6] hisi_sas: add hisi_sas_slot_abort() John Garry <john.garry@huawei.com> - 2016-02-16 16:50 +0100
          Re: [PATCH 2/6] hisi_sas: add hisi_sas_slot_abort() John Garry <john.garry@huawei.com> - 2016-02-18 10:40 +0100
    [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() John Garry <john.garry@huawei.com> - 2016-02-16 13:10 +0100
      Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() Hannes Reinecke <hare@suse.de> - 2016-02-16 16:40 +0100
        Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() John Garry <john.garry@huawei.com> - 2016-02-16 18:00 +0100
          Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() Hannes Reinecke <hare@suse.de> - 2016-02-18 08:50 +0100
            Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() John Garry <john.garry@huawei.com> - 2016-02-18 11:20 +0100
              Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() Hannes Reinecke <hare@suse.de> - 2016-02-18 11:40 +0100
                Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() John Garry <john.garry@huawei.com> - 2016-02-18 12:00 +0100
                  Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() John Garry <john.garry@huawei.com> - 2016-02-19 11:50 +0100
                    Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() Hannes Reinecke <hare@suse.de> - 2016-02-19 15:40 +0100
                      Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() John Garry <john.garry@huawei.com> - 2016-02-22 11:10 +0100
    [PATCH 3/6] hisi_sas: use slot abort in v1 hw John Garry <john.garry@huawei.com> - 2016-02-16 13:10 +0100
      Re: [PATCH 3/6] hisi_sas: use slot abort in v1 hw Hannes Reinecke <hare@suse.de> - 2016-02-16 16:40 +0100
        Re: [PATCH 3/6] hisi_sas: use slot abort in v1 hw John Garry <john.garry@huawei.com> - 2016-02-16 17:20 +0100
          Re: [PATCH 3/6] hisi_sas: use slot abort in v1 hw Hannes Reinecke <hare@suse.de> - 2016-02-18 08:20 +0100
            Re: [PATCH 3/6] hisi_sas: use slot abort in v1 hw John Garry <john.garry@huawei.com> - 2016-02-18 11:00 +0100
    [PATCH 4/6] hisi_sas: use slot abort in v2 hw John Garry <john.garry@huawei.com> - 2016-02-16 13:10 +0100
      Re: [PATCH 4/6] hisi_sas: use slot abort in v2 hw Hannes Reinecke <hare@suse.de> - 2016-02-16 16:40 +0100
        Re: [PATCH 4/6] hisi_sas: use slot abort in v2 hw John Garry <john.garry@huawei.com> - 2016-02-16 18:00 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1335605 — Re: [PATCH 3/6] hisi_sas: use slot abort in v1 hw

FromJohn Garry <john.garry@huawei.com>
Date2016-02-16 17:20 +0100
SubjectRe: [PATCH 3/6] hisi_sas: use slot abort in v1 hw
Message-ID<r2Qw2-1LV-31@gated-at.bofh.it>
In reply to#1335543
On 16/02/2016 15:31, Hannes Reinecke wrote:
> On 02/16/2016 01:22 PM, John Garry wrote:
>> When TRANS_TX_CREDIT_TIMEOUT_ERR or
>> TRANS_TX_CLOSE_NORMAL_ERR errors occur for a
>> command, the command should be re-attempted.
>>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>>   drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 22 ++++++++++++++++++----
>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
>> index ce5f65d..34f71a1c 100644
>> --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
>> @@ -1118,9 +1118,8 @@ static int prep_ssp_v1_hw(struct hisi_hba *hisi_hba,
>>   }
>>
>>   /* by default, task resp is complete */
>> -static void slot_err_v1_hw(struct hisi_hba *hisi_hba,
>> -			   struct sas_task *task,
>> -			   struct hisi_sas_slot *slot)
>> +static void slot_err_v1_hw(struct hisi_hba *hisi_hba, struct sas_task *task,
>> +			   struct hisi_sas_slot *slot, int *abort_slot)
>>   {
>>   	struct task_status_struct *ts = &task->task_status;
>>   	struct hisi_sas_err_record_v1 *err_record = slot->status_buffer;
>> @@ -1212,6 +1211,14 @@ static void slot_err_v1_hw(struct hisi_hba *hisi_hba,
>>   			ts->stat = SAS_NAK_R_ERR;
>>   			break;
>>   		}
>> +		case TRANS_TX_CREDIT_TIMEOUT_ERR:
>> +		case TRANS_TX_CLOSE_NORMAL_ERR:
>> +		{
>> +			/* This will request a retry */
>> +			ts->stat = SAS_QUEUE_FULL;
>> +			++(*abort_slot);
>> +			break;
>> +		}
>>   		default:
>>   		{
>>   			ts->stat = SAM_STAT_CHECK_CONDITION;
>> @@ -1317,8 +1324,14 @@ static int slot_complete_v1_hw(struct hisi_hba *hisi_hba,
>>
>>   	if (cmplt_hdr_data & CMPLT_HDR_ERR_RCRD_XFRD_MSK &&
>>   		!(cmplt_hdr_data & CMPLT_HDR_RSPNS_XFRD_MSK)) {
>> +		int abort_slot = 0;
>>
>> -		slot_err_v1_hw(hisi_hba, task, slot);
>> +		slot_err_v1_hw(hisi_hba, task, slot,  &abort_slot);
>> +		if (unlikely(abort_slot)) {
>> +			queue_work(hisi_hba->wq, &slot->abort_slot);
>> +			sts = ts->stat;
>> +			goto out_1;
>> +		}
>>   		goto out;
>>   	}
>>
> What is the 'abort_slot' variable for?
> Currently it's just a counter, no?
> So why the weird pointer passing?
>
> And it does feel weird. Apparently the driver does get a message,
> but still has to abort the command. Why?
> Isn't the message an indicator that the command has been aborted?
>
> Cheers,
>
> Hannes
>

I'll paste some more code for convenience and to help clarify:

static int slot_complete_v1_hw(struct hisi_hba *hisi_hba,
                    struct hisi_sas_slot *slot, int abort)
{
...

     if (cmplt_hdr_data & CMPLT_HDR_ERR_RCRD_XFRD_MSK &&
         !(cmplt_hdr_data & CMPLT_HDR_RSPNS_XFRD_MSK)) {
         int abort_slot = 0;

         slot_err_v1_hw(hisi_hba, task, slot,  &abort_slot);
         if (unlikely(abort_slot)) { /* check if we need to abort the 
task */
             queue_work(hisi_hba->wq, &slot->abort_slot);
             sts = ts->stat;
             goto out_1;
         }
         goto out;
     }

  ...

out:
     if (sas_dev && sas_dev->running_req)
         sas_dev->running_req--;

     hisi_sas_slot_task_free(hisi_hba, task, slot);
     sts = ts->stat;

     if (task->task_done)
         task->task_done(task);
out_1:

     return sts;
}

Variable abort_slot is really a boolean flag which can be set in 
slot_err_v1_hw(). When error TRANS_TX_CREDIT_TIMEOUT_ERR or 
TRANS_TX_CLOSE_NORMAL_ERR occurs in the slot, abort_slot is set. In this 
case we don't immediately complete the task (goto out and call 
hisi_sas_slot_task_free() and task->task_done()), but instead queue the 
task to be aborted in the device before completing (call queue_work() 
and then goto out_1).
When hisi_sas_slot_abort() [patch #2] runs in the workqueue for the 
task, it first aborts the task in the device with a TMF, and then 
completes the task. Finally the status (SAS_QUEUE_FULL) is passed back 
to SCSI framework, which will request a retry for the scsi command.

This is the method our hw people recommended to handle these types of 
errors.

Hope this explains,
Cheers,
John

[toc] | [prev] | [next] | [standalone]

#1337095 — Re: [PATCH 3/6] hisi_sas: use slot abort in v1 hw

FromHannes Reinecke <hare@suse.de>
Date2016-02-18 08:20 +0100
SubjectRe: [PATCH 3/6] hisi_sas: use slot abort in v1 hw
Message-ID<r3r2y-229-9@gated-at.bofh.it>
In reply to#1335605
On 02/16/2016 05:13 PM, John Garry wrote:
> On 16/02/2016 15:31, Hannes Reinecke wrote:
>> On 02/16/2016 01:22 PM, John Garry wrote:
>>> When TRANS_TX_CREDIT_TIMEOUT_ERR or
>>> TRANS_TX_CLOSE_NORMAL_ERR errors occur for a
>>> command, the command should be re-attempted.
>>>
>>> Signed-off-by: John Garry <john.garry@huawei.com>
>>> ---
>>>   drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 22 ++++++++++++++++++----
>>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
>>> b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
>>> index ce5f65d..34f71a1c 100644
>>> --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
>>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
>>> @@ -1118,9 +1118,8 @@ static int prep_ssp_v1_hw(struct hisi_hba
>>> *hisi_hba,
>>>   }
>>>
>>>   /* by default, task resp is complete */
>>> -static void slot_err_v1_hw(struct hisi_hba *hisi_hba,
>>> -               struct sas_task *task,
>>> -               struct hisi_sas_slot *slot)
>>> +static void slot_err_v1_hw(struct hisi_hba *hisi_hba, struct
>>> sas_task *task,
>>> +               struct hisi_sas_slot *slot, int *abort_slot)
>>>   {
>>>       struct task_status_struct *ts = &task->task_status;
>>>       struct hisi_sas_err_record_v1 *err_record =
>>> slot->status_buffer;
>>> @@ -1212,6 +1211,14 @@ static void slot_err_v1_hw(struct hisi_hba
>>> *hisi_hba,
>>>               ts->stat = SAS_NAK_R_ERR;
>>>               break;
>>>           }
>>> +        case TRANS_TX_CREDIT_TIMEOUT_ERR:
>>> +        case TRANS_TX_CLOSE_NORMAL_ERR:
>>> +        {
>>> +            /* This will request a retry */
>>> +            ts->stat = SAS_QUEUE_FULL;
>>> +            ++(*abort_slot);
>>> +            break;
>>> +        }
>>>           default:
>>>           {
>>>               ts->stat = SAM_STAT_CHECK_CONDITION;
>>> @@ -1317,8 +1324,14 @@ static int slot_complete_v1_hw(struct
>>> hisi_hba *hisi_hba,
>>>
>>>       if (cmplt_hdr_data & CMPLT_HDR_ERR_RCRD_XFRD_MSK &&
>>>           !(cmplt_hdr_data & CMPLT_HDR_RSPNS_XFRD_MSK)) {
>>> +        int abort_slot = 0;
>>>
>>> -        slot_err_v1_hw(hisi_hba, task, slot);
>>> +        slot_err_v1_hw(hisi_hba, task, slot,  &abort_slot);
>>> +        if (unlikely(abort_slot)) {
>>> +            queue_work(hisi_hba->wq, &slot->abort_slot);
>>> +            sts = ts->stat;
>>> +            goto out_1;
>>> +        }
>>>           goto out;
>>>       }
>>>
>> What is the 'abort_slot' variable for?
>> Currently it's just a counter, no?
>> So why the weird pointer passing?
>>
>> And it does feel weird. Apparently the driver does get a message,
>> but still has to abort the command. Why?
>> Isn't the message an indicator that the command has been aborted?
>>
>> Cheers,
>>
>> Hannes
>>
> 
> I'll paste some more code for convenience and to help clarify:
> 
> static int slot_complete_v1_hw(struct hisi_hba *hisi_hba,
>                    struct hisi_sas_slot *slot, int abort)
> {
> ...
> 
>     if (cmplt_hdr_data & CMPLT_HDR_ERR_RCRD_XFRD_MSK &&
>         !(cmplt_hdr_data & CMPLT_HDR_RSPNS_XFRD_MSK)) {
>         int abort_slot = 0;
> 
>         slot_err_v1_hw(hisi_hba, task, slot,  &abort_slot);
>         if (unlikely(abort_slot)) { /* check if we need to abort the
> task */
>             queue_work(hisi_hba->wq, &slot->abort_slot);
>             sts = ts->stat;
>             goto out_1;
>         }
>         goto out;
>     }
> 
>  ...
> 
> out:
>     if (sas_dev && sas_dev->running_req)
>         sas_dev->running_req--;
> 
>     hisi_sas_slot_task_free(hisi_hba, task, slot);
>     sts = ts->stat;
> 
>     if (task->task_done)
>         task->task_done(task);
> out_1:
> 
>     return sts;
> }
> 
> Variable abort_slot is really a boolean flag which can be set in
> slot_err_v1_hw(). When error TRANS_TX_CREDIT_TIMEOUT_ERR or
> TRANS_TX_CLOSE_NORMAL_ERR occurs in the slot, abort_slot is set. In
> this case we don't immediately complete the task (goto out and call
> hisi_sas_slot_task_free() and task->task_done()), but instead queue
> the task to be aborted in the device before completing (call
> queue_work() and then goto out_1).
So why not make slot_err_vi_hw() a boolean and have abort_slot as
the return value?

> When hisi_sas_slot_abort() [patch #2] runs in the workqueue for the
> task, it first aborts the task in the device with a TMF, and then
> completes the task. Finally the status (SAS_QUEUE_FULL) is passed
> back to SCSI framework, which will request a retry for the scsi
> command.
> 
> This is the method our hw people recommended to handle these types
> of errors.
> 
Ok, sure, that does explain it.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [next] | [standalone]

#1337211 — Re: [PATCH 3/6] hisi_sas: use slot abort in v1 hw

FromJohn Garry <john.garry@huawei.com>
Date2016-02-18 11:00 +0100
SubjectRe: [PATCH 3/6] hisi_sas: use slot abort in v1 hw
Message-ID<r3txn-3CY-9@gated-at.bofh.it>
In reply to#1337095
>>>>    /* by default, task resp is complete */
>>>> -static void slot_err_v1_hw(struct hisi_hba *hisi_hba,
>>>> -               struct sas_task *task,
>>>> -               struct hisi_sas_slot *slot)
>>>> +static void slot_err_v1_hw(struct hisi_hba *hisi_hba, struct
>>>> sas_task *task,
>>>> +               struct hisi_sas_slot *slot, int *abort_slot)
>>>>    {
>>>>        struct task_status_struct *ts = &task->task_status;
>>>>        struct hisi_sas_err_record_v1 *err_record =
>>>> slot->status_buffer;
>>>> @@ -1212,6 +1211,14 @@ static void slot_err_v1_hw(struct hisi_hba
>>>> *hisi_hba,
>>>>                ts->stat = SAS_NAK_R_ERR;
>>>>                break;
>>>>            }
>>>> +        case TRANS_TX_CREDIT_TIMEOUT_ERR:
>>>> +        case TRANS_TX_CLOSE_NORMAL_ERR:
>>>> +        {
>>>> +            /* This will request a retry */
>>>> +            ts->stat = SAS_QUEUE_FULL;
>>>> +            ++(*abort_slot);
>>>> +            break;
>>>> +        }
>>>>            default:
>>>>            {
>>>>                ts->stat = SAM_STAT_CHECK_CONDITION;
>>>> @@ -1317,8 +1324,14 @@ static int slot_complete_v1_hw(struct
>>>> hisi_hba *hisi_hba,
>>>>
>>>>        if (cmplt_hdr_data & CMPLT_HDR_ERR_RCRD_XFRD_MSK &&
>>>>            !(cmplt_hdr_data & CMPLT_HDR_RSPNS_XFRD_MSK)) {
>>>> +        int abort_slot = 0;
>>>>
>>>> -        slot_err_v1_hw(hisi_hba, task, slot);
>>>> +        slot_err_v1_hw(hisi_hba, task, slot,  &abort_slot);
>>>> +        if (unlikely(abort_slot)) {
>>>> +            queue_work(hisi_hba->wq, &slot->abort_slot);
>>>> +            sts = ts->stat;
>>>> +            goto out_1;
>>>> +        }
>>>>            goto out;
>>>>        }
>>>>
>>
>> static int slot_complete_v1_hw(struct hisi_hba *hisi_hba,
>>                     struct hisi_sas_slot *slot, int abort)
>> {
>> ...
>>
>>      if (cmplt_hdr_data & CMPLT_HDR_ERR_RCRD_XFRD_MSK &&
>>          !(cmplt_hdr_data & CMPLT_HDR_RSPNS_XFRD_MSK)) {
>>          int abort_slot = 0;
>>
>>          slot_err_v1_hw(hisi_hba, task, slot,  &abort_slot);
>>          if (unlikely(abort_slot)) { /* check if we need to abort the
>> task */
>>              queue_work(hisi_hba->wq, &slot->abort_slot);
>>              sts = ts->stat;
>>              goto out_1;
>>          }
>>          goto out;
>>      }

>>
>> Variable abort_slot is really a boolean flag which can be set in
>> slot_err_v1_hw(). When error TRANS_TX_CREDIT_TIMEOUT_ERR or
>> TRANS_TX_CLOSE_NORMAL_ERR occurs in the slot, abort_slot is set. In
>> this case we don't immediately complete the task (goto out and call
>> hisi_sas_slot_task_free() and task->task_done()), but instead queue
>> the task to be aborted in the device before completing (call
>> queue_work() and then goto out_1).
> So why not make slot_err_vi_hw() a boolean and have abort_slot as
> the return value?
>

I am not happy that this function should return anything, more 
specifically only whether the task should be aborted. I would be 
concerned that if it did return this value then it may have to be 
changed later on if the code needs to be changed. However it would make 
the code a bit tighter now.

Alternatively I could pass a pointer to a boolean (sounds bad), or even 
inline slot_err_v1_hw() in slot_complete_v1_hw(), as this is the only 
place it is called from.

Cheers,
John

[toc] | [prev] | [next] | [standalone]

#1335313 — [PATCH 4/6] hisi_sas: use slot abort in v2 hw

FromJohn Garry <john.garry@huawei.com>
Date2016-02-16 13:10 +0100
Subject[PATCH 4/6] hisi_sas: use slot abort in v2 hw
Message-ID<r2MC7-7Er-29@gated-at.bofh.it>
In reply to#1335305
When TRANS_TX_ERR_FRAME_TXED error occurs for
a command, the command should be re-attempted.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 58e1956..2bf93079b 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1190,7 +1190,8 @@ static void sata_done_v2_hw(struct hisi_hba *hisi_hba, struct sas_task *task,
 /* by default, task resp is complete */
 static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
 			   struct sas_task *task,
-			   struct hisi_sas_slot *slot)
+			   struct hisi_sas_slot *slot,
+			   int *abort_slot)
 {
 	struct task_status_struct *ts = &task->task_status;
 	struct hisi_sas_err_record_v2 *err_record = slot->status_buffer;
@@ -1299,6 +1300,13 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
 			ts->stat = SAS_DATA_UNDERRUN;
 			break;
 		}
+		case TRANS_TX_ERR_FRAME_TXED:
+		{
+			/* This will request a retry */
+			ts->stat = SAS_QUEUE_FULL;
+			++(*abort_slot);
+			break;
+		}
 		case TRANS_TX_OPEN_FAIL_WITH_IT_NEXUS_LOSS:
 		case TRANS_TX_ERR_PHY_NOT_ENABLE:
 		case TRANS_TX_OPEN_CNX_ERR_BY_OTHER:
@@ -1491,11 +1499,17 @@ slot_complete_v2_hw(struct hisi_hba *hisi_hba, struct hisi_sas_slot *slot,
 
 	if ((complete_hdr->dw0 & CMPLT_HDR_ERX_MSK) &&
 		(!(complete_hdr->dw0 & CMPLT_HDR_RSPNS_XFRD_MSK))) {
+		int abort_slot = 0;
 		dev_dbg(dev, "%s slot %d has error info 0x%x\n",
 			__func__, slot->cmplt_queue_slot,
 			complete_hdr->dw0 & CMPLT_HDR_ERX_MSK);
 
-		slot_err_v2_hw(hisi_hba, task, slot);
+		slot_err_v2_hw(hisi_hba, task, slot, &abort_slot);
+		if (unlikely(abort_slot)) {
+			queue_work(hisi_hba->wq, &slot->abort_slot);
+			sts = ts->stat;
+			goto out_1;
+		}
 		goto out;
 	}
 
@@ -1555,6 +1569,7 @@ out:
 
 	if (task->task_done)
 		task->task_done(task);
+out_1:
 
 	return sts;
 }
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1335551 — Re: [PATCH 4/6] hisi_sas: use slot abort in v2 hw

FromHannes Reinecke <hare@suse.de>
Date2016-02-16 16:40 +0100
SubjectRe: [PATCH 4/6] hisi_sas: use slot abort in v2 hw
Message-ID<r2PTm-1fi-59@gated-at.bofh.it>
In reply to#1335313
On 02/16/2016 01:22 PM, John Garry wrote:
> When TRANS_TX_ERR_FRAME_TXED error occurs for
> a command, the command should be re-attempted.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> index 58e1956..2bf93079b 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> @@ -1190,7 +1190,8 @@ static void sata_done_v2_hw(struct hisi_hba *hisi_hba, struct sas_task *task,
>  /* by default, task resp is complete */
>  static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
>  			   struct sas_task *task,
> -			   struct hisi_sas_slot *slot)
> +			   struct hisi_sas_slot *slot,
> +			   int *abort_slot)
>  {
>  	struct task_status_struct *ts = &task->task_status;
>  	struct hisi_sas_err_record_v2 *err_record = slot->status_buffer;
> @@ -1299,6 +1300,13 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
>  			ts->stat = SAS_DATA_UNDERRUN;
>  			break;
>  		}
> +		case TRANS_TX_ERR_FRAME_TXED:
> +		{
> +			/* This will request a retry */
> +			ts->stat = SAS_QUEUE_FULL;
> +			++(*abort_slot);
> +			break;
> +		}
>  		case TRANS_TX_OPEN_FAIL_WITH_IT_NEXUS_LOSS:
>  		case TRANS_TX_ERR_PHY_NOT_ENABLE:
>  		case TRANS_TX_OPEN_CNX_ERR_BY_OTHER:
> @@ -1491,11 +1499,17 @@ slot_complete_v2_hw(struct hisi_hba *hisi_hba, struct hisi_sas_slot *slot,
>  
>  	if ((complete_hdr->dw0 & CMPLT_HDR_ERX_MSK) &&
>  		(!(complete_hdr->dw0 & CMPLT_HDR_RSPNS_XFRD_MSK))) {
> +		int abort_slot = 0;
>  		dev_dbg(dev, "%s slot %d has error info 0x%x\n",
>  			__func__, slot->cmplt_queue_slot,
>  			complete_hdr->dw0 & CMPLT_HDR_ERX_MSK);
>  
> -		slot_err_v2_hw(hisi_hba, task, slot);
> +		slot_err_v2_hw(hisi_hba, task, slot, &abort_slot);
> +		if (unlikely(abort_slot)) {
> +			queue_work(hisi_hba->wq, &slot->abort_slot);
> +			sts = ts->stat;
> +			goto out_1;
> +		}
>  		goto out;
>  	}
>  
Again this weird 'abort_slot' pointer reshuffling.
Care to clarify?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [next] | [standalone]

#1335641 — Re: [PATCH 4/6] hisi_sas: use slot abort in v2 hw

FromJohn Garry <john.garry@huawei.com>
Date2016-02-16 18:00 +0100
SubjectRe: [PATCH 4/6] hisi_sas: use slot abort in v2 hw
Message-ID<r2R8K-21y-27@gated-at.bofh.it>
In reply to#1335551
On 16/02/2016 15:32, Hannes Reinecke wrote:
> On 02/16/2016 01:22 PM, John Garry wrote:
>> When TRANS_TX_ERR_FRAME_TXED error occurs for
>> a command, the command should be re-attempted.
>>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>>   drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 19 +++++++++++++++++--
>>   1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>> index 58e1956..2bf93079b 100644
>> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>> @@ -1190,7 +1190,8 @@ static void sata_done_v2_hw(struct hisi_hba *hisi_hba, struct sas_task *task,
>>   /* by default, task resp is complete */
>>   static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
>>   			   struct sas_task *task,
>> -			   struct hisi_sas_slot *slot)
>> +			   struct hisi_sas_slot *slot,
>> +			   int *abort_slot)
>>   {
>>   	struct task_status_struct *ts = &task->task_status;
>>   	struct hisi_sas_err_record_v2 *err_record = slot->status_buffer;
>> @@ -1299,6 +1300,13 @@ static void slot_err_v2_hw(struct hisi_hba *hisi_hba,
>>   			ts->stat = SAS_DATA_UNDERRUN;
>>   			break;
>>   		}
>> +		case TRANS_TX_ERR_FRAME_TXED:
>> +		{
>> +			/* This will request a retry */
>> +			ts->stat = SAS_QUEUE_FULL;
>> +			++(*abort_slot);
>> +			break;
>> +		}
>>   		case TRANS_TX_OPEN_FAIL_WITH_IT_NEXUS_LOSS:
>>   		case TRANS_TX_ERR_PHY_NOT_ENABLE:
>>   		case TRANS_TX_OPEN_CNX_ERR_BY_OTHER:
>> @@ -1491,11 +1499,17 @@ slot_complete_v2_hw(struct hisi_hba *hisi_hba, struct hisi_sas_slot *slot,
>>
>>   	if ((complete_hdr->dw0 & CMPLT_HDR_ERX_MSK) &&
>>   		(!(complete_hdr->dw0 & CMPLT_HDR_RSPNS_XFRD_MSK))) {
>> +		int abort_slot = 0;
>>   		dev_dbg(dev, "%s slot %d has error info 0x%x\n",
>>   			__func__, slot->cmplt_queue_slot,
>>   			complete_hdr->dw0 & CMPLT_HDR_ERX_MSK);
>>
>> -		slot_err_v2_hw(hisi_hba, task, slot);
>> +		slot_err_v2_hw(hisi_hba, task, slot, &abort_slot);
>> +		if (unlikely(abort_slot)) {
>> +			queue_work(hisi_hba->wq, &slot->abort_slot);
>> +			sts = ts->stat;
>> +			goto out_1;
>> +		}
>>   		goto out;
>>   	}
>>
> Again this weird 'abort_slot' pointer reshuffling.
> Care to clarify?
>
> Cheers,
>
> Hannes
>

Hopefully my explanation for patch #3 will help clarify here, as the 
code is functionally the same.

Thanks,
John

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web