On 5/3/17 1:55 PM, Bryant G. Ly wrote:
On 5/3/17 11:38 AM, Bryant G. Ly wrote:
Hi Nick,
On 5/2/17 10:43 PM, Nicholas A. Bellinger wrote:
On Tue, 2017-05-02 at 13:54 -0500, Bryant G. Ly wrote:
The driver is sending a response to the actual scsi op that was
aborted by an abort task TM, while LIO is sending a response to
the abort task TM.
ibmvscsis_tgt does not send the response to the client until
release_cmd time. The reason for this was because if we did it
at queue_status time, then the client would be free to reuse the
tag for that command, but we're still using the tag until the
command is released at release_cmd time, so we chose to delay
sending the response until then. That then caused this issue, because
release_cmd is always called, even if queue_status is not.
SCSI spec says that the initiator that sends the abort task
TM NEVER gets a response to the aborted op and with the current
code it will send a response. Thus this fix will remove that response
if the TAS bit is set.
Cc: <stable@xxxxxxxxxxxxxxx> # v4.8+
Signed-off-by: Bryant G. Ly <bryantly@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Tyrel Datwyler <tyreld@xxxxxxxxxxxxxxxxxx>
---
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 66
++++++++++++++++++++++----------
1 file changed, 45 insertions(+), 21 deletions(-)
Applied, with a small update to the last sentence of the commit log wrt
to 'if ABORTED && !TAS bit is set'.
Thanks Bryant + Tyrel.
So I have been running tests on this patch extensively and it seems
like after running 2 hrs of consecutive aborts it fails again with
the same problem of driver sending a response to the actual scsi op.
It looks like when LIO processes the abort and if
(!__target_check_io_state(se_cmd, se_sess, 0)) then rsp gets set as
TMR_TASK_DOES_NOT_EXIST.
With that response the CMD_T_ABORTED state is not set, so then the
ibmvscsis driver still sends that duplicate response.
I think the best solution would be in driver when queue_tm_rsp gets
called and when the driver builds the response to check for the
TMR_TASK_DOES_NOT_EXIST and set our own flag.
Then we would also check for that flag, so it would be:
if ((cmd->se_cmd.transport_state & CMD_T_ABORTED &&
!(cmd->se_cmd.transport_state & CMD_T_TAS)) || cmd->flags &
CMD_ABORTED) {
This will ensure in the CMD_T_ABORTED w/o CMD_T_TAS scenarios are
handled and the ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST case.
-Bryant
Hi Nick,
You can ignore the email about the patch being broken. The script that
was used for testing isn't right.
So the patch is working as intended now, where on an abort the client
does not receive an extra response to the actual scsi op.
Thanks,
Bryant Ly
Sorry Nick, but can you hold off on this patch. We are still getting extra responses but in a small timing window.
The majority of the aborts work but if we get an abort between LIO calling queue_state and LIO calling release_cmd, we get an extra response.
Working on a solution, and will still have to do the tests where we basically send aborts over and over on the client btwn random timing widows.
-Bryant