Re: [PATCH 04/36] target: Does tmr notify on aborted command

Thinh Nguyen
2022-07-12T02:57:16Z

On 7/11/2022, Dmitry Bogdanov wrote:
> On Fri, Jul 08, 2022 at 11:11:37PM +0000, Thinh Nguyen wrote:
>> «Внимание! Данное письмо от внешнего адресата!»
>>
>> On 7/7/2022, Dmitry Bogdanov wrote:
>>> Hi Thinh,
>>>
>>> On Wed, Jul 06, 2022 at 04:34:49PM -0700, Thinh Nguyen wrote:
>>>> If the tmr_notify is not implemented, simply execute a generic command
>>>> completion to notify the command abort.
>>> Why? What are you trying to fix?
>> If tmr_notify() is not implemented (which most don't), then the user
>> won't get notified of the command completion.
> tmr_notify is for transport drivers (iblock/pscsi/user) - transport of
> IOs to the real storage device. Not for trasport of incoming SCSI
> messages - that is a frontend driver in TCM terms.
> So, USB frontend driver has nothing to do with transport->tmr_notify().

I see. Thanks clarifying the terminology here.

>> I was trying to directly notify the user via target_complete_cmd(). It
>> may not be the right way to handle this, any advise?
> Frontend drivers are notified of the aborted task twice:
> 1. The incoming TMF in frontend driver; usually a frontend driver do not
> do anything here, just pass TMF to TCM Core.
> 2. TCM Core makrs the command as "to be aborted".
> cmd->transport_state |= CMD_T_ABORTED;
> 2. TCM Core checks that command is to be aborted when IO is not started
> yet or IO is completed:
> * target_execute_cmd(start of handling SCSI cmd),
> * target_compete_cmd (backend device completes IO),
> * transport_generic_request_failure (some generic request to send a
> failure response)
> And calls target_handle_abort() which calls
> cmd->se_tfo->aborted_task(cmd) to notify frontend driver that it will
> not be asked to send response to the command and it may do some cleanup
> if needed.
>
> There are two possible continuous processes in a cmd lifecycle:
> 1. Data IN (several responses to initiator)
> TCM Core receives a data from transport (backstore device) and passes
> it to frontend driver. Frontend driver is responsible to send it to the
> initiator. Probably, it may check that cmd is aborted to break sending,
> but nobody do that.
> 2. Data OUT (several requests from initiators)
> Data from DataOUT is collected by frontend driver to pass it to TCM
> Core in target_submit_cmd. TCM Core will abort the cmd at that moment.
>
> There is no interface in TCM Core to notify Frontend driver to stop
> those continuous processes. Probably, because of differences in fronted
> protocol standards.
> For example, iSCSI tunes that behaviour by some negotiatable session
> parameter. Current kernel iSCSI driver does not support that parameter.

Thank you for the detail explanation. Really appreciate it.

BR,
Thinh

>>>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>>>> ---
>>>> drivers/target/target_core_tmr.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
>>>> index 7a7e24069ba7..2af80d0998bf 100644
>>>> --- a/drivers/target/target_core_tmr.c
>>>> +++ b/drivers/target/target_core_tmr.c
>>>> @@ -14,6 +14,7 @@
>>>> #include <linux/spinlock.h>
>>>> #include <linux/list.h>
>>>> #include <linux/export.h>
>>>> +#include <scsi/scsi_proto.h>
>>>>
>>>> #include <target/target_core_base.h>
>>>> #include <target/target_core_backend.h>
>>>> @@ -150,6 +151,9 @@ void core_tmr_abort_task(
>>>> if (dev->transport->tmr_notify)
>>>> dev->transport->tmr_notify(dev, TMR_ABORT_TASK,
>>>> &aborted_list);
>>>> + else
>>>> + target_complete_cmd(se_cmd,
>>>> + SAM_STAT_TASK_ABORTED);
>>> That is wrong and breaks a command lifecycle and command kref counting.
>>> target_complete_cmd is used to be called by a backend driver.
>>>> list_del_init(&se_cmd->state_list);
>>>> target_put_cmd_and_wait(se_cmd);