On Sun, 2017-04-02 at 15:38 -0700, Nicholas A. Bellinger wrote: > No. Your assumption is incorrect wrt transport_generic_free_cmd() > having a wrong return value during LUN_RESET. > > It's incorrect because when iscsit_free_cmd() is called with > shutdown=true resulting in transport_generic_free_cmd() with > wait_for_tasks=true, target_wait_free_cmd() checks for CMD_T_ABORTED to > determine if se_cmd has been aborted by target_core_tmr.c logic, and > returns aborted=true back up to transport_generic_free_cmd(). > > When transport_generic_free_cmd() gets aborted=true, it waits for > se_cmd->cmd_wait_comp to finish and calls cmd->se_tfo->release_cmd() > to release se_cmd back to the pre-allocated session pool. > > At this point, transport_generic_free_cmd() with aborted=true must > return '1' it's caller, signaling iscsit_free_cmd() to not attempt to > perform the extra __iscsi_free_cmd() or target_put_sess_cmd(), because > se_cmd has already been released back to the session pool. Hello Nic, Today the iSCSI target driver relies on transport_generic_free_cmd() to figure out whether target_put_sess_cmd() has been called once or twice for a SCSI command. transport_generic_free_cmd() relies on the command reference count and the "aborted" flag to perform that check. I think that's an unfortunate design choice because it makes it impossible to use the command reference count outside any context where it is used today (calling .release_cmd() upon final put / TMF handling), e.g. to simplify the implementation of target_wait_for_sess_cmds(). This choice also makes it necessary to perform direct calls to .release_cmd() from several functions, something that is really unfortunate. I think it is important to make all that code easier to understand and to verify. Hence my proposal to introduce a second reference count that tracks the number of references a target driver holds on a SCSI command. Such a patch has been posted before. See also "target: Introduce a target driver reference count per command" (February 9, 2017, https://www.spinics.net/lists/target-devel/msg14487.html). Is it OK for you if I rework my most recently posted iSCSI target driver patch series on top of that patch? Bart.-- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
