* [PATCH v2 0/4] target: Updates related to UASP @ 2022-07-19 2:06 Thinh Nguyen 2022-07-19 2:07 ` [PATCH v2 2/4] target: Implement TMR_ABORT_TASK_SET Thinh Nguyen 2022-07-19 2:07 ` [PATCH v2 3/4] target: Properly set Sense Data Length of CHECK CONDITION Thinh Nguyen 0 siblings, 2 replies; 6+ messages in thread From: Thinh Nguyen @ 2022-07-19 2:06 UTC (permalink / raw) To: linux-kernel, linux-scsi, target-devel, Martin K. Petersen Cc: John Youn, Thinh Nguyen, linux-usb Here are some changes splitted from the series below [*]: "[PATCH 00/36] usb: gadget: f_tcm: Enhance UASP driver" Cc: linux-usb@vger.kernel.org Changes in v2: - From review, remove the following patches either because they are incorrect or not needed: target: Don't drain empty list target: Does tmr notify on aborted command target: Don't remove command too early target: Return Function Complete target: Don't do tmr_notify on empty aborted list target: Refactor core_tmr_abort_task target: Don't respond TMR_LUN_DOES_NOT_EXIST for all TMR failure target: Introduce target_submit_tmr_fail_response target: Include INQUIRY length - Minor fixes to the remaining patches [*] https://lore.kernel.org/linux-usb/cover.1657149962.git.Thinh.Nguyen@synopsys.com/ Thinh Nguyen (4): target: Handle MI_REPORT_SUPPORTED_OPERATION_CODES target: Implement TMR_ABORT_TASK_SET target: Properly set Sense Data Length of CHECK CONDITION target: Properly set Sense data length when copy sense drivers/target/target_core_alua.c | 70 ++++++++++++++++++++++++++ drivers/target/target_core_alua.h | 2 + drivers/target/target_core_spc.c | 14 +++++- drivers/target/target_core_tmr.c | 16 ++++-- drivers/target/target_core_transport.c | 21 ++++++-- 5 files changed, 111 insertions(+), 12 deletions(-) base-commit: 88a15fbb47db483d06b12b1ae69f114b96361a96 -- 2.28.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/4] target: Implement TMR_ABORT_TASK_SET 2022-07-19 2:06 [PATCH v2 0/4] target: Updates related to UASP Thinh Nguyen @ 2022-07-19 2:07 ` Thinh Nguyen 2022-07-19 15:56 ` Mike Christie 2022-07-19 2:07 ` [PATCH v2 3/4] target: Properly set Sense Data Length of CHECK CONDITION Thinh Nguyen 1 sibling, 1 reply; 6+ messages in thread From: Thinh Nguyen @ 2022-07-19 2:07 UTC (permalink / raw) To: linux-kernel, linux-scsi, target-devel, Martin K. Petersen Cc: John Youn, Thinh Nguyen, linux-usb 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(-) 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; -- 2.28.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/4] target: Implement TMR_ABORT_TASK_SET 2022-07-19 2:07 ` [PATCH v2 2/4] target: Implement TMR_ABORT_TASK_SET Thinh Nguyen @ 2022-07-19 15:56 ` Mike Christie 2022-07-19 23:05 ` Thinh Nguyen 2022-07-20 15:41 ` Konstantin Shelekhin 0 siblings, 2 replies; 6+ messages in thread From: Mike Christie @ 2022-07-19 15:56 UTC (permalink / raw) To: Thinh Nguyen, linux-kernel, linux-scsi, target-devel, Martin K. Petersen Cc: John Youn, linux-usb 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. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/4] target: Implement TMR_ABORT_TASK_SET 2022-07-19 15:56 ` Mike Christie @ 2022-07-19 23:05 ` Thinh Nguyen 2022-07-20 15:41 ` Konstantin Shelekhin 1 sibling, 0 replies; 6+ messages in thread From: Thinh Nguyen @ 2022-07-19 23:05 UTC (permalink / raw) To: Mike Christie, Thinh Nguyen, linux-kernel, linux-scsi, target-devel, Martin K. Petersen Cc: John Youn, linux-usb 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. >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/4] target: Implement TMR_ABORT_TASK_SET 2022-07-19 15:56 ` Mike Christie 2022-07-19 23:05 ` Thinh Nguyen @ 2022-07-20 15:41 ` Konstantin Shelekhin 1 sibling, 0 replies; 6+ messages in thread From: Konstantin Shelekhin @ 2022-07-20 15:41 UTC (permalink / raw) To: Mike Christie Cc: Thinh Nguyen, linux-kernel, linux-scsi, target-devel, Martin K. Petersen, John Youn, linux-usb 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] target: Properly set Sense Data Length of CHECK CONDITION 2022-07-19 2:06 [PATCH v2 0/4] target: Updates related to UASP Thinh Nguyen 2022-07-19 2:07 ` [PATCH v2 2/4] target: Implement TMR_ABORT_TASK_SET Thinh Nguyen @ 2022-07-19 2:07 ` Thinh Nguyen 1 sibling, 0 replies; 6+ messages in thread From: Thinh Nguyen @ 2022-07-19 2:07 UTC (permalink / raw) To: linux-kernel, linux-scsi, target-devel, Martin K. Petersen Cc: John Youn, Felipe Balbi, Greg KH, Thinh Nguyen, linux-usb CHECK CONDITION returns sense data, and sense data is minimum 8 bytes long plus additional sense data length in the data buffer. Don't just set the allocated buffer length to the cmd->scsi_sense_length and check the sense data for that. See SPC4-r37 section 4.5.2.1. Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> --- Changes in v2: - Make sure it doesn't overflow and properly pass TRANSPORT_SENSE_BUFFER to scsi_set_sense_information() drivers/target/target_core_transport.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 8c386142ef90..db53b326048d 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -3459,12 +3459,19 @@ static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason) cmd->se_cmd_flags |= SCF_EMULATED_TASK_SENSE; cmd->scsi_status = SAM_STAT_CHECK_CONDITION; - cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER; + scsi_build_sense_buffer(desc_format, buffer, key, asc, ascq); if (sd->add_sense_info) WARN_ON_ONCE(scsi_set_sense_information(buffer, - cmd->scsi_sense_length, + TRANSPORT_SENSE_BUFFER, cmd->sense_info) < 0); + /* + * CHECK CONDITION returns sense data, and sense data is minimum 8 + * bytes long plus additional Sense Data Length. + * See SPC4-r37 section 4.5.2.1. + */ + cmd->scsi_sense_length = min_t(u16, buffer[7] + 8, + TRANSPORT_SENSE_BUFFER); } int -- 2.28.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-20 15:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-19 2:06 [PATCH v2 0/4] target: Updates related to UASP Thinh Nguyen 2022-07-19 2:07 ` [PATCH v2 2/4] target: Implement TMR_ABORT_TASK_SET Thinh Nguyen 2022-07-19 15:56 ` Mike Christie 2022-07-19 23:05 ` Thinh Nguyen 2022-07-20 15:41 ` Konstantin Shelekhin 2022-07-19 2:07 ` [PATCH v2 3/4] target: Properly set Sense Data Length of CHECK CONDITION Thinh Nguyen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).