Re: [PATCH 05/36] target: Don't remove command too early

Dmitry Bogdanov
2022-07-07T13:16:12Z

Hi Thinh,

On Wed, Jul 06, 2022 at 04:34:55PM -0700, Thinh Nguyen wrote:
> A command completion is asynchronous, regardless if an abort command is
> executed. Don't just free the command before its completion. Similarly,
> a TMR command is not completed until its response is completed. The
> freeing of the command can be done by the target user through
> target_generic_free_cmd().
>
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
> drivers/target/target_core_transport.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 7838dc20f713..105d3b0e470f 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -836,10 +836,6 @@ static void target_handle_abort(struct se_cmd *cmd)
> }
>
> WARN_ON_ONCE(kref_read(&cmd->cmd_kref) == 0);
> -
> - transport_lun_remove_cmd(cmd);
> -
> - transport_cmd_check_stop_to_fabric(cmd);
> }
>
> static void target_abort_work(struct work_struct *work)
> @@ -3553,9 +3549,6 @@ static void target_tmr_work(struct work_struct *work)
> goto aborted;
>
> cmd->se_tfo->queue_tm_rsp(cmd);
> -
> - transport_lun_remove_cmd(cmd);
> - transport_cmd_check_stop_to_fabric(cmd);
> return;
>
> aborted:
Those functions are not about to free the command.
transport_lun_remove_cmd is for remove command from the state/tmr list.
transport_cmd_check_stop_to_fabric is for notify a fabric driver to
decrease the command kref that it owns. And eventually to wake
target_put_cmd_and_wait() in core_tmr_abort_task().

Those functions do always are called after a final response has been sent
(STATUS, CHECK_CONDITION,etc).
Those functions do not break the abort functionality. But this patch
does.