| Message ID | 0004ae8cc8650a32f2aaab0ef9ee3b6e6eb6b69c.1657149962.git.Thinh.Nguyen@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: f_tcm: Enhance UASP driver | expand |
Hi Thinh, On Wed, Jul 06, 2022 at 04:35:26PM -0700, Thinh Nguyen wrote: > 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. TCM Core does not support Task Sets, there is no list of commands per I_T nexus. Your patch aborts all commands in all I_T nexuses for the particular backstore device. That is defenitely not according to SAM. > > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > --- > 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 3e73f60319d5..e77721db1ea9 100644 > --- a/drivers/target/target_core_tmr.c > +++ b/drivers/target/target_core_tmr.c > @@ -132,11 +132,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); > @@ -159,7 +161,11 @@ void core_tmr_abort_task( > target_put_cmd_and_wait(se_cmd); > > atomic_long_inc(&dev->aborts_complete); > - goto exit; > + > + if (tmr->function == TMR_ABORT_TASK) > + goto exit; > + > + 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 cbd876e44cf0..bc1e4a7c4538 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;
On Thu, Jul 07, 2022 at 10:40:18PM +0300, Dmitry Bogdanov wrote: > Hi Thinh, > > On Wed, Jul 06, 2022 at 04:35:26PM -0700, Thinh Nguyen wrote: > > 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. > TCM Core does not support Task Sets, there is no list of commands per > I_T nexus. Your patch aborts all commands in all I_T nexuses for the > particular backstore device. That is defenitely not according to SAM. Sorry, there is a check against se_sess, so this patch looks good actually. > > > > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > > --- > > 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 3e73f60319d5..e77721db1ea9 100644 > > --- a/drivers/target/target_core_tmr.c > > +++ b/drivers/target/target_core_tmr.c > > @@ -132,11 +132,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); > > @@ -159,7 +161,11 @@ void core_tmr_abort_task( > > target_put_cmd_and_wait(se_cmd); > > > > atomic_long_inc(&dev->aborts_complete); > > - goto exit; > > + > > + if (tmr->function == TMR_ABORT_TASK) > > + goto exit; > > + > > + 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 cbd876e44cf0..bc1e4a7c4538 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;
On 7/8/2022, Dmitry Bogdanov wrote: > On Thu, Jul 07, 2022 at 10:40:18PM +0300, Dmitry Bogdanov wrote: >> Hi Thinh, >> >> On Wed, Jul 06, 2022 at 04:35:26PM -0700, Thinh Nguyen wrote: >>> 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. >> TCM Core does not support Task Sets, there is no list of commands per >> I_T nexus. Your patch aborts all commands in all I_T nexuses for the >> particular backstore device. That is defenitely not according to SAM. > Sorry, there is a check against se_sess, so this patch looks good > actually. Ok. This is my first time reviewing and working on the target core and f_tcm. There are quite a few specs and codes to go through and understand. If I missed something, please continue to help point it out. Thanks for all the reviews (I know there are many patches). BR, Thinh >>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> >>> --- >>> 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 3e73f60319d5..e77721db1ea9 100644 >>> --- a/drivers/target/target_core_tmr.c >>> +++ b/drivers/target/target_core_tmr.c >>> @@ -132,11 +132,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); >>> @@ -159,7 +161,11 @@ void core_tmr_abort_task( >>> target_put_cmd_and_wait(se_cmd); >>> >>> atomic_long_inc(&dev->aborts_complete); >>> - goto exit; >>> + >>> + if (tmr->function == TMR_ABORT_TASK) >>> + goto exit; >>> + >>> + 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 cbd876e44cf0..bc1e4a7c4538 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;
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index 3e73f60319d5..e77721db1ea9 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -132,11 +132,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); @@ -159,7 +161,11 @@ void core_tmr_abort_task( target_put_cmd_and_wait(se_cmd); atomic_long_inc(&dev->aborts_complete); - goto exit; + + if (tmr->function == TMR_ABORT_TASK) + goto exit; + + 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 cbd876e44cf0..bc1e4a7c4538 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;
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> --- drivers/target/target_core_tmr.c | 16 +++++++++++----- drivers/target/target_core_transport.c | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-)