Hi Mike,
> > This is just a nit. Maybe just do:
> >
> > struct se_device *dev = NULL;
> > unsigned long flags;
> >
> > if (!(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB))
> > return;
> >
> > dev = cmd->se_tmr_req->tmr_dev;
> > spin_lock_irqsave(&dev->se_tmr_lock, flags);
> > list_del_init(&cmd->se_tmr_req->tmr_list);
> > spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
> >
> This might be wrong. I thought when you moved the deletion to
> transport_cmd_check_stop_to_fabric, we would always have a dev set.
> But in core_tmr_abort_task there is that transport_lookup_tmr_lun
> check. If that is a valid check in there and it could be NULL in
> the path:
> transport_generic_handle_tmr -> target_handle_abort ->
> ransport_cmd_check_stop_to_fabric
>
> then we would hit a NULL pointer. I'm not sure how we would get a NULL
> dev there though. The driver would have to not use the standard
> target_submit_tmr or be iscsi and not call transport_lookup_tmr_lun.
In theory it can do some out-of-tree transport driver, ok I will keep a guard
in target_remove_from_tmr_list().
>
> One issue with the patch though is if iscsit_tmr_abort_task fails then we don't
> call transport_cmd_check_stop_to_fabric, so the tmr will be stuck on the list.
Yes, I see. I am going to use this patch to solve it:
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -2142,7 +2142,7 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
* TMR TASK_REASSIGN.
*/
iscsit_add_cmd_to_response_queue(cmd, conn, cmd->i_state);
- target_put_sess_cmd(&cmd->se_cmd);
+ transport_generic_free_cmd(&cmd->se_cmd, false);
return 0;
}
EXPORT_SYMBOL(iscsit_handle_task_mgt_cmd);
BR,
Dmitry
