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

Dmitry Bogdanov
2022-07-11T10:34:43Z

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 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.

>
> Thanks,
> 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);
>