On 01/06/2016 11:42 PM, Nicholas A. Bellinger wrote:
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.
Hello Nic,
Please note that two comments from that e-mail have already been
addressed. The two comments about session shutdown have been addressed
through patch "[PATCH 09/21] target: Simplify session shutdown code". I
will also address the other comments before I repost this patch series.
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
