On Sat, 2011-10-08 at 20:57 -0400, Christoph Hellwig wrote: > On Sat, Oct 08, 2011 at 02:38:28PM -0700, Nicholas A. Bellinger wrote: > > ->check_stop_free() was originally added for tcm_loop, which is able to > > release it's descriptor in the main callback path, eg: > > scsi_cmnd->scsi_done is called directly within ->queue_data_in() and > > ->queue_status, and we don't have to wait for a fabric acknowledge > > before releasing.. > > > > tcm_fc is using ->check_stop_free() in the same manner as tcm_loop, to > > directly release se_cmd immediately via transport_generic_free_cmd() > > What does releasing us earlier buy in practice? Unless it has real > benefits showing up in performance numbers I really hate shortcuts > like that. > For tcm_loop there is no other processing context for the completed descriptor to be released in. The scsi_cmnd->scsi_done() callback has been invoked, and the associated se_cmd is ready to be released. Not releasing at this point would involve extra work and/or an extra context switch per released descriptor. > > > (2) what the point of SCF_SE_LUN_CMD is. As a first a !se_cmd->se_lun > > > check should give us the same information. But more importantly > > > why we would ever allow a command without a LUN to stay around, > > > and more importantly be on core target lists that could lead to > > > a free. > > > > This was originally used by iscsi-target to identify which descriptors > > had a se_lun association, but eventually bled into target-core. In > > modern target core code this ends up signaling when a se_cmd is > > associated with a TMR, but is for all intensive purposes unnecessary in > > target-core. > > I'd love to see it going away ASAP as it makes life in the command > put/release area a whole lot simpler, but at this point I don't feel > comfortable enough with the iscsi target code yet to do it myself. > Yep, i'll look at sorting this out with the removal of se_cmd->transport_wait_for_tasks(). Thanks, --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
