diff mbox series

[v2,2/4] target: Implement TMR_ABORT_TASK_SET

Message ID 7d31722a7e07bc24ea37b5841a17545003eeddb4.1658195608.git.Thinh.Nguyen@synopsys.com (mailing list archive)
State Changes Requested
Headers show
Series target: Updates related to UASP | expand

Commit Message

Thinh Nguyen July 19, 2022, 2:07 a.m. UTC
Task ABORT TASK SET function is required by SCSI transport protocol
standards (SAM-4 r14 section 7.3). It is similar to ABORT TASK
function, but it applies to all commands received on a specified I_T
nexus rather than a specific referenced command. Modify
core_tmr_abort_task() to support TMR_ABORT_TASK_SET.

Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
 Changes in v2:
 - None

 drivers/target/target_core_tmr.c       | 16 +++++++++++-----
 drivers/target/target_core_transport.c |  2 +-
 2 files changed, 12 insertions(+), 6 deletions(-)

Comments

Mike Christie July 19, 2022, 3:56 p.m. UTC | #1
On 7/18/22 9:07 PM, Thinh Nguyen wrote:
> Task ABORT TASK SET function is required by SCSI transport protocol

What OS is using this and how do they use it? For the latter, does the
OS try an abort for each cmd first, then try an abort task set if the
aborts fail (does fail mean get a response that indicates failure and
also does a timeout count)? Or does it start with the abort task set?

I'm asking because it looks like if it does an abort first, then the
abort task set will always return TMR_TASK_DOES_NOT_EXIST. For the abort
we will remove the cmds from the state_list so if the abort task set runs
after the initiator has tried to abort all the commands it will never
find any.

> standards (SAM-4 r14 section 7.3). It is similar to ABORT TASK
> function, but it applies to all commands received on a specified I_T
> nexus rather than a specific referenced command. Modify
> core_tmr_abort_task() to support TMR_ABORT_TASK_SET.
>
Thinh Nguyen July 19, 2022, 11:05 p.m. UTC | #2
On 7/19/2022, Mike Christie wrote:
> On 7/18/22 9:07 PM, Thinh Nguyen wrote:
>> Task ABORT TASK SET function is required by SCSI transport protocol
> What OS is using this and how do they use it? For the latter, does the
> OS try an abort for each cmd first, then try an abort task set if the
> aborts fail (does fail mean get a response that indicates failure and
> also does a timeout count)? Or does it start with the abort task set?

It's not from any real driver. It's from the USB Compliant Verification 
(https://www.usb.org/document-library/usb3cv). It uses the command for 
UASP compliant test.

The test only ever aborts a single command at a time, so I can't confirm 
your following questions. The SAM4-r14 wasn't clear on those questions 
either.

> I'm asking because it looks like if it does an abort first, then the
> abort task set will always return TMR_TASK_DOES_NOT_EXIST. For the abort
> we will remove the cmds from the state_list so if the abort task set runs
> after the initiator has tried to abort all the commands it will never
> find any.

I didn't notice since I dropped a patch where I removed the 
TMR_TASK_DOES_NOT_EXIST and UASP converts this to RC_TMF_COMPLETE. UASP 
respond to FUNCTION COMPLETE with RC_TMF_COMPLETE. I'll can make a fix 
to that.

If there's any suggestion to implement this, please advise.

Thanks,
Thinh

>> standards (SAM-4 r14 section 7.3). It is similar to ABORT TASK
>> function, but it applies to all commands received on a specified I_T
>> nexus rather than a specific referenced command. Modify
>> core_tmr_abort_task() to support TMR_ABORT_TASK_SET.
>>
Konstantin Shelekhin July 20, 2022, 3:41 p.m. UTC | #3
On Tue, Jul 19, 2022 at 10:56:07AM -0500, Mike Christie wrote:
> «Внимание! Данное письмо от внешнего адресата!»
> 
> On 7/18/22 9:07 PM, Thinh Nguyen wrote:
> > Task ABORT TASK SET function is required by SCSI transport protocol
> 
> What OS is using this and how do they use it? For the latter, does the
> OS try an abort for each cmd first, then try an abort task set if the
> aborts fail (does fail mean get a response that indicates failure and
> also does a timeout count)? Or does it start with the abort task set?

AIX IIRC. However, this feature also requires valid bits in one of the
VPD pages.
diff mbox series

Patch

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index bac111456fa1..1ea72e15f872 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -131,11 +131,13 @@  void core_tmr_abort_task(
 				continue;
 
 			ref_tag = se_cmd->tag;
-			if (tmr->ref_task_tag != ref_tag)
-				continue;
+			if (tmr->function == TMR_ABORT_TASK) {
+				if (tmr->ref_task_tag != ref_tag)
+					continue;
 
-			pr_err("ABORT_TASK: Found referenced %s task_tag: %llu\n",
-			       se_cmd->se_tfo->fabric_name, ref_tag);
+				pr_err("ABORT_TASK: Found referenced %s task_tag: %llu\n",
+				       se_cmd->se_tfo->fabric_name, ref_tag);
+			}
 
 			spin_lock(&se_sess->sess_cmd_lock);
 			rc = __target_check_io_state(se_cmd, se_sess, 0);
@@ -158,7 +160,11 @@  void core_tmr_abort_task(
 			       ref_tag);
 			tmr->response = TMR_FUNCTION_COMPLETE;
 			atomic_long_inc(&dev->aborts_complete);
-			return;
+
+			if (tmr->function == TMR_ABORT_TASK)
+				return;
+
+			spin_lock_irqsave(&dev->queues[i].lock, flags);
 		}
 		spin_unlock_irqrestore(&dev->queues[i].lock, flags);
 	}
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 7838dc20f713..8c386142ef90 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -3519,9 +3519,9 @@  static void target_tmr_work(struct work_struct *work)
 
 	switch (tmr->function) {
 	case TMR_ABORT_TASK:
+	case TMR_ABORT_TASK_SET:
 		core_tmr_abort_task(dev, tmr, cmd->se_sess);
 		break;
-	case TMR_ABORT_TASK_SET:
 	case TMR_CLEAR_ACA:
 	case TMR_CLEAR_TASK_SET:
 		tmr->response = TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED;