Re: [PATCH 10/36] target: Implement TMR_ABORT_TASK_SET

Thinh Nguyen
2022-07-09T00:46:50Z

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;