diff mbox series

[10/36] target: Implement TMR_ABORT_TASK_SET

Message ID 0004ae8cc8650a32f2aaab0ef9ee3b6e6eb6b69c.1657149962.git.Thinh.Nguyen@synopsys.com
State Superseded
Headers show
Series usb: gadget: f_tcm: Enhance UASP driver | expand

Commit Message

Thinh Nguyen July 6, 2022, 11:35 p.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>
---
 drivers/target/target_core_tmr.c       | 16 +++++++++++-----
 drivers/target/target_core_transport.c |  2 +-
 2 files changed, 12 insertions(+), 6 deletions(-)

Comments

Dmitry Bogdanov July 7, 2022, 7:40 p.m. UTC | #1 
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;
Dmitry Bogdanov July 8, 2022, 9:50 a.m. UTC | #2 
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;
Thinh Nguyen July 9, 2022, 12:46 a.m. UTC | #3 
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 mbox series

Patch

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;