Hi Mike, >>> --- 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; >>> } >> >> Doh. I see how I got all confused. I guess this path leaks the lun_ref >> taken by transport_lookup_tmr_lun. It looks like an old issue and >> nothing to do with your patch. >> I'm not sure if we are supposed to be calling >> transport_generic_free_cmd twice. It looks like it works ok, because your patch added the "cmd->se_lun = NULL" >> in transport_lun_remove_cmd, so we won't do a double list deletion. >> It feels dirty though. I can feel Bart saying, "Mike did you see the >> comment at the top of the function". :) >> >> Maybe it's best to more cleanly unwind what was setup in the rror >> path. I think we can fix lun_ref leak too. >> >> So instead of doing transport_generic_free_cmd above do >> transport_lun_remove_cmd to match/undo the transport_lookup_tmr_lun call in iscsit_handle_task_mgt_cmd? > >Shoot. I'm all over the place. I think the root issue is my original comment on the v1 patch was wrong. >On a failure we would still do: >iscsit_free_cmd -> transport_generic_free_cmd -> transport_lun_remove_cmd >right? So we don't need any change in the iscsi target. It should all just work. iscsit_free_cmd will be called only at response completion(next incoming iscsi command): iscsit_ack_from_expstatsn -> iscsit_free_cmd. That produces some unacceptable delay of lun unlinking from cmd. There is a bug report to the similar behavior: http://lkml.iu.edu/hypermail/linux/kernel/2002.0/05272.html Because of that complain, the commit 83f85b8ec305, that solves the same crash as I am fixing, was reverted. So, this piece of patch has some indirect relation :) I will extract it to a separate patch in the coming patchset on TMF handling. BR, Dmitry
