Hi Bart, First, I wanted to say thanks for shepherding patches into upstream over the holidays and into the new year. My focus has been almost exclusively productizing target code for stable kernel releases the last months, so it's really a nice suprise to see that youself and other folks have been merging target patches to mainline. Comments below. On Wed, 2017-02-01 at 16:58 -0800, Bart Van Assche wrote: > The most important changes in this patch series are: > * ABORT and LUN RESET handling is made synchronous. > * LUN lookup during ABORT handling is moved from the tcm_qla2xxx > into the target core. > * A deadlock in the XCOPY implementation is fixed. > > These patches have been tested against the iSCSI, SRP and qla2xxx > target drivers. > > The changes compared to v1 are: > - Introduced a new workqueue per session for task management functions. > - Renamed tmr_wq into alua_wq. > > Please review the patches that have not yet been reviewed, > > Thanks, > > Bart. > > Bart Van Assche (36): > target/iscsi: Fix indentation in iscsi_target_start_negotiation() > target/iscsi: Fix spelling of "perform" > target/iscsi: Fix spelling of "reallegiance" > target/iscsi: Introduce a helper function for TMF translation > target/iscsi: Fix iSCSI task reassignment handling > target: Remove se_tmr_req.tmr_lun > target: Make core_tmr_abort_task() consider all commands > target: Rework the transport_alloc_session_tags() error path > target: Introduce a new workqueue for TMF processing > target: Rename tmr_wq into alua_wq > target: Make it possible to specify I_T nexus for SCSI abort > tcm_qla2xxx: Let the target core look up the LUN of the aborted cmd > target: Correct transport_wait_for_tasks() documentation > target: Remove an overly chatty debug message > target: Add a missing target_put_nacl() call > target: Stop execution if CMD_T_STOP has been set > target: Fix a reference leak in transport_cmd_check_stop_to_fabric() > target: Inline transport_cmd_check_stop() > target: Make ABORT and LUN RESET handling synchronous So, I see that we're back to discussing the significant changes to how ABORT and LUN_RESET work. First off, I agree with you that this is an area that is difficult to read, and certainly is ripe for improvement. However as I've voiced loudly in the past, there are a number of issues I have with the approach in these specific patches, as well as what I think are still some fundamental misunderstandings about how the existing code functions. Due to this misunderstandings, there are a number of regressions this series as-is introduces That said, I'm going to try my best to explain line-by-line the issues in the individual patches, and give some guidance on alternatives to clean-up and simplify this particular area. I'd like to avoid short responses this particularly complex technical topics, which increases the frustration level on both sides. So if something in-line I've highlighted doesn't make sense or you think is incorrect, please respond in-line to comment and I will clarify. > target: Simplify session shutdown code > target: Remove the SCF_SEND_DELAYED_TAS command flag > target: Inline transport_check_aborted_status() > target: Remove the write_pending_status() callback function > target: Remove several state tests from TMF code > target: Remove command flag CMD_T_BUSY > target: Simplify LUN RESET implementation > target: Remove command flag CMD_T_DEV_ACTIVE > target: Remove command flag CMD_T_TAS > target: Reduce number of __transport_wait_for_tasks() arguments > target: Remove unused arguments from __target_check_io_state() > target: Change return type of transport_wait_for_tasks() into void > target: Inline transport_put_cmd() > target: Inline transport_lun_remove_cmd() > target: Move target_remove_from_state_list() into > target_release_cmd_kref() > target: Introduce target_get_device() and target_put_device() > target: Avoid that XCOPY commands trigger a deadlock Thank you, --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
