On Sat, 2011-10-08 at 20:46 -0400, Christoph Hellwig wrote: > > After taking another look at the 'session_reinstatement' bit, I think > > it's legacy cruft for iscsi-target, and it's safe to drop it entirely. > > > > Along with your original series, i'm including the following patch. > > Makes sense. In addition you can also remove the bool returns from > transport_put_cmd and transport_generic_free_cmd now. > Yep, removing this now.. > Another thing I would really like to to is to clean > upcmd->transport_wait_for_tasks. It is a fairly odd use of a callback > in every cmd, which essentially replaces a try-state: > > - noop if explicitly set by assigning transport_nop_wait_for_tasks > - finish sutdown if explicitly assigning transport_lun_wait_for_tasks > - > I think you mean transport_generic_wait_for_tasks() here.. > In generic code we only call it with both flags sent to 0/false from > transport_generic_free_cmd, but the iSCSI target uses all variants, > and passes the arguments. > Now that the session_reinstatement bit is gone, iscsi-target will only call this to signal 'Stop the command', or 'Stop the command, and release it directly via transport_generic_free_cmd()'. > It seems to me like we should simple have flags in the se_cmdn to > not wait, and one inside the iscsi code for the current semantics if > neither callback is set (if that is indeed still needed), and then > call the 0/0 variant directly from transport_generic_free_cmd. > I think it should be OK to call transport_generic_free_cmd() directly here, get rid of the se_cmd->transport_wait_for_tasks() pointer, and the 'int remove' parameter from transport_generic_wait_for_tasks() as well. The one thing that needs to be verifed is if calling transport_generic_wait_for_tasks() after the core_dec_lacl_count() and transport_lun_remove_cmd() in transport_generic_free_cmd() is going to be problematic. > Note that the iSCSI code callers make heavy use of SCF_SE_LUN_CMD > discussed in the next mail and really could benefit from factoring > and coument the callers to too, but I don't really know the code > very well. > Yep, i'll have a look at these as well and will see if (most) of the direct usage of SCF_SE_LUN_CMD around ->transport_wait_for_tasks() calls can go away as well.. --nab -- 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
