On Tue, 2016-01-05 at 14:50 +0100, Bart Van Assche wrote: > Instead of invoking target driver callback functions from the > context that handles an abort or LUN RESET task management function, > only set the abort flag from that context and perform the actual > abort handling from the context of the regular command processing > flow. This approach has the following advantages: > - The task management function code becomes much easier to read and > to verify since the number of potential race conditions against > the command processing flow is strongly reduced. > - It is no longer needed to store the command state into the command > itself since that information is no longer needed from the context > where a task management function is processed. > > Notes: > - The target_remove_from_state_list() call in transport_cmd_check_stop() > has been moved up such that it is called without t_state_lock held. > This is necessary to avoid triggering lock inversion against > core_tmr_drain_state_list(), which now locks execute_task_lock and > t_state_lock in that order. > - With this patch applied the CMD_T_ABORTED flag is checked at two points > by the target core: just before local execution of a command starts > (see also target_execute_cmd()) and also just before the response is > sent (see also target_complete_cmd()). > > Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx> > Cc: Andy Grover <agrover@xxxxxxxxxx> > Cc: Christoph Hellwig <hch@xxxxxx> > Cc: Sagi Grimberg <sagig@xxxxxxxxxxxx> > --- > drivers/target/target_core_internal.h | 3 - > drivers/target/target_core_tmr.c | 93 ++++++++++--------- > drivers/target/target_core_transport.c | 159 ++++++++++----------------------- > include/target/target_core_base.h | 5 +- > 4 files changed, 102 insertions(+), 158 deletions(-) > Grrrrrrr. You've utterly ignored everything mentioned in the review from November: http://www.spinics.net/lists/target-devel/msg11057.html I was hoping that you'd do the right thing as requested, and go ahead and fix the existing TMR LUN_RESET referencing counting bugs first separate from the other changes in this patch. As mentioned earlier, these need to be back-ported to stable and mixing them in with other random stuff is not going to be an option. That said, don't even bother to re-posting this until you read the previous comments, and take the time to understand the issues involved and respond with more than a two sentence reply. I'm fixing the TMR LUN_RESET bugs now as-is based on the other patch-set from QLogic, and will submit myself for v4.5-rc. Really, if you can't even take the time to address a single one of previous comments, don't expect me to look at this again until you've put in the prerequisite work required for a significant change such as this. -- 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
