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

Groups > linux.kernel > #1469326 > unrolled thread

[PATCH v2 0/8] hisi_sas internal abort support

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/8] hisi_sas internal abort support John Garry <john.garry@huawei.com> - 2016-08-24 12:50 +0200
    [PATCH v2 7/8] hisi_sas: add TMF success check John Garry <john.garry@huawei.com> - 2016-08-24 12:50 +0200
    [PATCH v2 6/8] hisi_sas: fail tmf task prep when port detached John Garry <john.garry@huawei.com> - 2016-08-24 13:00 +0200
    [PATCH v2 8/8] hisi_sas: update version to 1.6 John Garry <john.garry@huawei.com> - 2016-08-24 13:00 +0200
    [PATCH v2 5/8] hisi_sas: add v2 hw slot complete internal abort support John Garry <john.garry@huawei.com> - 2016-08-24 13:00 +0200
      Re: [PATCH v2 5/8] hisi_sas: add v2 hw slot complete internal abort  support Hannes Reinecke <hare@suse.de> - 2016-08-24 15:10 +0200
        Re: [PATCH v2 5/8] hisi_sas: add v2 hw slot complete internal abort  support John Garry <john.garry@huawei.com> - 2016-08-24 16:10 +0200
          Re: [PATCH v2 5/8] hisi_sas: add v2 hw slot complete internal abort  support Hannes Reinecke <hare@suse.de> - 2016-08-24 16:40 +0200

#1469326 — [PATCH v2 0/8] hisi_sas internal abort support

This patchset introduces support for the internal abort
feature for the HiSilicon SAS controller.

The internal abort feature allows commands which are active
in the controller to be aborted before being sent to the
slave device.

Only support will be added for v2 HW since v1 HW has issues
in supporting internal abort feature.

Differences to v1 patchset:
- Add MACRO for internal abort mode and description
  of internal abort API

John Garry (8):
  hisi_sas: add internal abort main code
  hisi_sas: for internal abort in hisi_sas_dev_gone()
  hisi_sas: add internal abort to hisi_sas_abort_task()
  hisi_sas: add prep_abort_v2_hw()
  hisi_sas: add v2 hw slot complete internal abort support
  hisi_sas: fail tmf task prep when port detached
  hisi_sas: add TMF success check
  hisi_sas: update version to 1.6

 drivers/scsi/hisi_sas/hisi_sas.h       |  10 +-
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 190 +++++++++++++++++++++++++++++++--
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c |  64 +++++++++++
 3 files changed, 256 insertions(+), 8 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1469327 — [PATCH v2 7/8] hisi_sas: add TMF success check

When a tmf is issued, various response codes can be
returned from the target. For a query tmf the
response may be TMF_RESP_FUNC_COMPLETE or
TMF_RESP_FUNC_SUCC.
Add a condition for TMF_RESP_FUNC_SUCC to
hisi_sas_exec_internal_tmf_task().
This affects query tmf, as the result is success
the returned value was for failure.

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

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 73f581f..85c73d3 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -744,6 +744,12 @@ static int hisi_sas_exec_internal_tmf_task(struct domain_device *device,
 		}
 
 		if (task->task_status.resp == SAS_TASK_COMPLETE &&
+			task->task_status.stat == TMF_RESP_FUNC_SUCC) {
+			res = TMF_RESP_FUNC_SUCC;
+			break;
+		}
+
+		if (task->task_status.resp == SAS_TASK_COMPLETE &&
 		      task->task_status.stat == SAS_DATA_UNDERRUN) {
 			/* no error, but return the number of bytes of
 			 * underrun
-- 
1.9.1

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

#1469329 — [PATCH v2 6/8] hisi_sas: fail tmf task prep when port detached

When the port is detached we cannot execute a TMF,
as there can be no device attached to the port.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 280a19d..73f581f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -204,7 +204,7 @@ static int hisi_sas_task_prep(struct sas_task *task, struct hisi_hba *hisi_hba,
 		return rc;
 	}
 	port = device->port->lldd_port;
-	if (port && !port->port_attached && !tmf) {
+	if (port && !port->port_attached) {
 		if (sas_protocol_ata(task->task_proto)) {
 			struct task_status_struct *ts = &task->task_status;
 
-- 
1.9.1

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

#1469332 — [PATCH v2 8/8] hisi_sas: update version to 1.6

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 8311360..ca55ec2 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -23,7 +23,7 @@
 #include <scsi/sas_ata.h>
 #include <scsi/libsas.h>
 
-#define DRV_VERSION "v1.5"
+#define DRV_VERSION "v1.6"
 
 #define HISI_SAS_MAX_PHYS	9
 #define HISI_SAS_MAX_QUEUES	32
-- 
1.9.1

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

#1469335 — [PATCH v2 5/8] hisi_sas: add v2 hw slot complete internal abort support

Add code in slot_complete_v2_hw() to deal with the
slots which have completed due to internal abort.

The status codes have the following meaning:
- STAT_IO_ABORTED: the IO has been aborted due to
internal abort, whether by device or individual
abort command
- STAT_IO_COMPLETE: internal abort command has
completed successfully for device or individual
abort command
- STAT_IO_NO_DEVICE: internal abort command has
completed for device but cannot find any IO
- STAT_IO_NOT_VALID: internal abort command has
completed for single command but could not
find the command

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

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index fec1675..bf9b693 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -227,6 +227,13 @@
 #define CMPLT_HDR_RSPNS_XFRD_MSK	(0x1 << CMPLT_HDR_RSPNS_XFRD_OFF)
 #define CMPLT_HDR_ERX_OFF		12
 #define CMPLT_HDR_ERX_MSK		(0x1 << CMPLT_HDR_ERX_OFF)
+#define CMPLT_HDR_ABORT_STAT_OFF	13
+#define CMPLT_HDR_ABORT_STAT_MSK	(0x7 << CMPLT_HDR_ABORT_STAT_OFF)
+/* abort_stat */
+#define STAT_IO_NOT_VALID		0x1
+#define STAT_IO_NO_DEVICE		0x2
+#define STAT_IO_COMPLETE		0x3
+#define STAT_IO_ABORTED			0x4
 /* dw1 */
 #define CMPLT_HDR_IPTT_OFF		0
 #define CMPLT_HDR_IPTT_MSK		(0xffff << CMPLT_HDR_IPTT_OFF)
@@ -1569,6 +1576,30 @@ slot_complete_v2_hw(struct hisi_hba *hisi_hba, struct hisi_sas_slot *slot,
 		goto out;
 	}
 
+	/* Use SAS+TMF status codes */
+	switch ((complete_hdr->dw0 & CMPLT_HDR_ABORT_STAT_MSK)
+			>> CMPLT_HDR_ABORT_STAT_OFF) {
+	case STAT_IO_ABORTED:
+		/* this io has been aborted by abort command */
+		ts->stat = SAS_ABORTED_TASK;
+		goto out;
+	case STAT_IO_COMPLETE:
+		/* internal abort command complete */
+		ts->stat = TMF_RESP_FUNC_COMPLETE;
+		goto out;
+	case STAT_IO_NO_DEVICE:
+		ts->stat = TMF_RESP_FUNC_COMPLETE;
+		goto out;
+	case STAT_IO_NOT_VALID:
+		/* abort single io, controller don't find
+		 * the io need to abort
+		 */
+		ts->stat = TMF_RESP_FUNC_FAILED;
+		goto out;
+	default:
+		break;
+	}
+
 	if ((complete_hdr->dw0 & CMPLT_HDR_ERX_MSK) &&
 		(!(complete_hdr->dw0 & CMPLT_HDR_RSPNS_XFRD_MSK))) {
 
-- 
1.9.1

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

#1469398 — Re: [PATCH v2 5/8] hisi_sas: add v2 hw slot complete internal abort support

On 08/24/2016 01:05 PM, John Garry wrote:
> Add code in slot_complete_v2_hw() to deal with the
> slots which have completed due to internal abort.
> 
> The status codes have the following meaning:
> - STAT_IO_ABORTED: the IO has been aborted due to
> internal abort, whether by device or individual
> abort command
> - STAT_IO_COMPLETE: internal abort command has
> completed successfully for device or individual
> abort command
> - STAT_IO_NO_DEVICE: internal abort command has
> completed for device but cannot find any IO
> - STAT_IO_NOT_VALID: internal abort command has
> completed for single command but could not
> find the command
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> index fec1675..bf9b693 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> @@ -227,6 +227,13 @@
>  #define CMPLT_HDR_RSPNS_XFRD_MSK	(0x1 << CMPLT_HDR_RSPNS_XFRD_OFF)
>  #define CMPLT_HDR_ERX_OFF		12
>  #define CMPLT_HDR_ERX_MSK		(0x1 << CMPLT_HDR_ERX_OFF)
> +#define CMPLT_HDR_ABORT_STAT_OFF	13
> +#define CMPLT_HDR_ABORT_STAT_MSK	(0x7 << CMPLT_HDR_ABORT_STAT_OFF)
> +/* abort_stat */
> +#define STAT_IO_NOT_VALID		0x1
> +#define STAT_IO_NO_DEVICE		0x2
> +#define STAT_IO_COMPLETE		0x3
> +#define STAT_IO_ABORTED			0x4
>  /* dw1 */
>  #define CMPLT_HDR_IPTT_OFF		0
>  #define CMPLT_HDR_IPTT_MSK		(0xffff << CMPLT_HDR_IPTT_OFF)
> @@ -1569,6 +1576,30 @@ slot_complete_v2_hw(struct hisi_hba *hisi_hba, struct hisi_sas_slot *slot,
>  		goto out;
>  	}
>  
> +	/* Use SAS+TMF status codes */
> +	switch ((complete_hdr->dw0 & CMPLT_HDR_ABORT_STAT_MSK)
> +			>> CMPLT_HDR_ABORT_STAT_OFF) {
> +	case STAT_IO_ABORTED:
> +		/* this io has been aborted by abort command */
> +		ts->stat = SAS_ABORTED_TASK;
> +		goto out;
> +	case STAT_IO_COMPLETE:
> +		/* internal abort command complete */
> +		ts->stat = TMF_RESP_FUNC_COMPLETE;
> +		goto out;
> +	case STAT_IO_NO_DEVICE:
> +		ts->stat = TMF_RESP_FUNC_COMPLETE;
> +		goto out;
> +	case STAT_IO_NOT_VALID:
> +		/* abort single io, controller don't find
> +		 * the io need to abort
> +		 */
> +		ts->stat = TMF_RESP_FUNC_FAILED;
> +		goto out;
Hmm. This will cause the SCSI EH to kick in.
And then, according to the description abort has succeeded, it's just
that for some reason the associated command couldn't be found.
So couldn't this be due to a race condition, and the command has in fact
been aborted correctly (and the code is just too slow acknowledging 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]

#1469479 — Re: [PATCH v2 5/8] hisi_sas: add v2 hw slot complete internal abort support

On 24/08/2016 13:59, Hannes Reinecke wrote:
> On 08/24/2016 01:05 PM, John Garry wrote:
>> Add code in slot_complete_v2_hw() to deal with the
>> slots which have completed due to internal abort.
>>
>> The status codes have the following meaning:
>> - STAT_IO_ABORTED: the IO has been aborted due to
>> internal abort, whether by device or individual
>> abort command
>> - STAT_IO_COMPLETE: internal abort command has
>> completed successfully for device or individual
>> abort command
>> - STAT_IO_NO_DEVICE: internal abort command has
>> completed for device but cannot find any IO
>> - STAT_IO_NOT_VALID: internal abort command has
>> completed for single command but could not
>> find the command
>>
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>>  drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>> index fec1675..bf9b693 100644
>> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>> @@ -227,6 +227,13 @@
>>  #define CMPLT_HDR_RSPNS_XFRD_MSK	(0x1 << CMPLT_HDR_RSPNS_XFRD_OFF)
>>  #define CMPLT_HDR_ERX_OFF		12
>>  #define CMPLT_HDR_ERX_MSK		(0x1 << CMPLT_HDR_ERX_OFF)
>> +#define CMPLT_HDR_ABORT_STAT_OFF	13
>> +#define CMPLT_HDR_ABORT_STAT_MSK	(0x7 << CMPLT_HDR_ABORT_STAT_OFF)
>> +/* abort_stat */
>> +#define STAT_IO_NOT_VALID		0x1
>> +#define STAT_IO_NO_DEVICE		0x2
>> +#define STAT_IO_COMPLETE		0x3
>> +#define STAT_IO_ABORTED			0x4
>>  /* dw1 */
>>  #define CMPLT_HDR_IPTT_OFF		0
>>  #define CMPLT_HDR_IPTT_MSK		(0xffff << CMPLT_HDR_IPTT_OFF)
>> @@ -1569,6 +1576,30 @@ slot_complete_v2_hw(struct hisi_hba *hisi_hba, struct hisi_sas_slot *slot,
>>  		goto out;
>>  	}
>>
>> +	/* Use SAS+TMF status codes */
>> +	switch ((complete_hdr->dw0 & CMPLT_HDR_ABORT_STAT_MSK)
>> +			>> CMPLT_HDR_ABORT_STAT_OFF) {
>> +	case STAT_IO_ABORTED:
>> +		/* this io has been aborted by abort command */
>> +		ts->stat = SAS_ABORTED_TASK;
>> +		goto out;
>> +	case STAT_IO_COMPLETE:
>> +		/* internal abort command complete */
>> +		ts->stat = TMF_RESP_FUNC_COMPLETE;
>> +		goto out;
>> +	case STAT_IO_NO_DEVICE:
>> +		ts->stat = TMF_RESP_FUNC_COMPLETE;
>> +		goto out;
>> +	case STAT_IO_NOT_VALID:
>> +		/* abort single io, controller don't find
>> +		 * the io need to abort
>> +		 */
>> +		ts->stat = TMF_RESP_FUNC_FAILED;
>> +		goto out;
> Hmm. This will cause the SCSI EH to kick in.
> And then, according to the description abort has succeeded, it's just
> that for some reason the associated command couldn't be found.
> So couldn't this be due to a race condition, and the command has in fact
> been aborted correctly (and the code is just too slow acknowledging it)?
>

Hi Hannes,

I'm not sure I fully get your question.

The internal abort would happen from the SCSI error handling. An example 
would be when the disk was not safely removed and some IO is still in 
flight. In this case the IO will timeout, SCSI EH starts, and we try to 
abort the command in LLDD, by TMF (which would fail) and internal abort.

For internal abort, if the abort command succeeds then 2 things happen:
- abort task completes with status STAT_IO_COMPLETE
- task which was aborted completes with status STAT_IO_ABORTED

If the command does not abort successfully then:
- abort task completes with status STAT_IO_NOT_VALID
- task which we wanted to be aborted does not complete and is probably 
still in the slave device

I hope that this makes it clear.

Thanks,
John

> Cheers,
>
> Hannes
>

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

#1469498 — Re: [PATCH v2 5/8] hisi_sas: add v2 hw slot complete internal abort support

On 08/24/2016 04:07 PM, John Garry wrote:
> On 24/08/2016 13:59, Hannes Reinecke wrote:
>> On 08/24/2016 01:05 PM, John Garry wrote:
>>> Add code in slot_complete_v2_hw() to deal with the
>>> slots which have completed due to internal abort.
>>>
>>> The status codes have the following meaning:
>>> - STAT_IO_ABORTED: the IO has been aborted due to
>>> internal abort, whether by device or individual
>>> abort command
>>> - STAT_IO_COMPLETE: internal abort command has
>>> completed successfully for device or individual
>>> abort command
>>> - STAT_IO_NO_DEVICE: internal abort command has
>>> completed for device but cannot find any IO
>>> - STAT_IO_NOT_VALID: internal abort command has
>>> completed for single command but could not
>>> find the command
>>>
>>> Signed-off-by: John Garry <john.garry@huawei.com>
>>> ---
>>>  drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 31
>>> +++++++++++++++++++++++++++++++
>>>  1 file changed, 31 insertions(+)
>>>
>>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>>> b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>>> index fec1675..bf9b693 100644
>>> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
>>> @@ -227,6 +227,13 @@
>>>  #define CMPLT_HDR_RSPNS_XFRD_MSK    (0x1 << CMPLT_HDR_RSPNS_XFRD_OFF)
>>>  #define CMPLT_HDR_ERX_OFF        12
>>>  #define CMPLT_HDR_ERX_MSK        (0x1 << CMPLT_HDR_ERX_OFF)
>>> +#define CMPLT_HDR_ABORT_STAT_OFF    13
>>> +#define CMPLT_HDR_ABORT_STAT_MSK    (0x7 << CMPLT_HDR_ABORT_STAT_OFF)
>>> +/* abort_stat */
>>> +#define STAT_IO_NOT_VALID        0x1
>>> +#define STAT_IO_NO_DEVICE        0x2
>>> +#define STAT_IO_COMPLETE        0x3
>>> +#define STAT_IO_ABORTED            0x4
>>>  /* dw1 */
>>>  #define CMPLT_HDR_IPTT_OFF        0
>>>  #define CMPLT_HDR_IPTT_MSK        (0xffff << CMPLT_HDR_IPTT_OFF)
>>> @@ -1569,6 +1576,30 @@ slot_complete_v2_hw(struct hisi_hba *hisi_hba,
>>> struct hisi_sas_slot *slot,
>>>          goto out;
>>>      }
>>>
>>> +    /* Use SAS+TMF status codes */
>>> +    switch ((complete_hdr->dw0 & CMPLT_HDR_ABORT_STAT_MSK)
>>> +            >> CMPLT_HDR_ABORT_STAT_OFF) {
>>> +    case STAT_IO_ABORTED:
>>> +        /* this io has been aborted by abort command */
>>> +        ts->stat = SAS_ABORTED_TASK;
>>> +        goto out;
>>> +    case STAT_IO_COMPLETE:
>>> +        /* internal abort command complete */
>>> +        ts->stat = TMF_RESP_FUNC_COMPLETE;
>>> +        goto out;
>>> +    case STAT_IO_NO_DEVICE:
>>> +        ts->stat = TMF_RESP_FUNC_COMPLETE;
>>> +        goto out;
>>> +    case STAT_IO_NOT_VALID:
>>> +        /* abort single io, controller don't find
>>> +         * the io need to abort
>>> +         */
>>> +        ts->stat = TMF_RESP_FUNC_FAILED;
>>> +        goto out;
>> Hmm. This will cause the SCSI EH to kick in.
>> And then, according to the description abort has succeeded, it's just
>> that for some reason the associated command couldn't be found.
>> So couldn't this be due to a race condition, and the command has in fact
>> been aborted correctly (and the code is just too slow acknowledging it)?
>>
> 
> Hi Hannes,
> 
> I'm not sure I fully get your question.
> 
> The internal abort would happen from the SCSI error handling. An example
> would be when the disk was not safely removed and some IO is still in
> flight. In this case the IO will timeout, SCSI EH starts, and we try to
> abort the command in LLDD, by TMF (which would fail) and internal abort.
> 
> For internal abort, if the abort command succeeds then 2 things happen:
> - abort task completes with status STAT_IO_COMPLETE
> - task which was aborted completes with status STAT_IO_ABORTED
> 
> If the command does not abort successfully then:
> - abort task completes with status STAT_IO_NOT_VALID
> - task which we wanted to be aborted does not complete and is probably
> still in the slave device
> 
> I hope that this makes it clear.
> 
Right, that answers it.

Reviewed-by: Hannes Reinecke <hare@suse.com>

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] | [standalone]

Back to top | Article view | linux.kernel

csiph-web