| Message ID | bb728dcbc73942cda3ae64d0d26195c9a284996d.1657149962.git.Thinh.Nguyen@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: f_tcm: Enhance UASP driver | expand |
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.
On 7/7/2022, Dmitry Bogdanov wrote: > 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. > > Looks like I misunderstood those functions' role. Thanks, Thinh
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:
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(-)