On 6/10/16 8:56 AM, Bryant G. Ly wrote:
On 2016-06-09 23:43, Nicholas A. Bellinger wrote:
Hey Bryant,
On Wed, 2016-06-08 at 08:54 -0500, Bryant G. Ly wrote:
If a Simple command is sent with an Unsupported SCSI Opcode (failure),
target_setup_cmd_from_cdb returns with TCM_UNSUPPORTED_SCSI_OPCODE
which then causes transport_generic_request_failure to decrement
simple_cmds, due to call to transport_complete_task_attr.
With this dev->simple_cmds or dev->dev_ordered_sync is now -1, not 0.
So when a subsequent command with an Ordered Task is sent, it causes
a hang, since dev->simple_cmds is at -1. Thus to prevent Unsupported
SCSI Opcode Failure from decrementing simple_cmds I added a check
to skip transport_complete_task_attr for sense_reaons that are
TCM_UNSUPPORTED_SCSI_OPCODE.
I dont know if this is a proper fix, but it worked for me, let me
know what you think Nick.
Signed-off-by: Bryant G. Ly <bryantly@xxxxxxxxxxxxxxxxxx>
---
drivers/target/target_core_transport.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/target/target_core_transport.c
b/drivers/target/target_core_transport.c
index 6c089af..4578fcf 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1684,7 +1684,8 @@ void transport_generic_request_failure(struct
se_cmd *cmd,
/*
* For SAM Task Attribute emulation for failed struct se_cmd
*/
- transport_complete_task_attr(cmd);
+ if (sense_reason != TCM_UNSUPPORTED_SCSI_OPCODE)
+ transport_complete_task_attr(cmd);
/*
* Handle special case for COMPARE_AND_WRITE failure, where the
* callback is expected to drop the per device ->caw_sem.
Just to confirm, this is a regression with the earlier patch here:
https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit/?id=15ce22fa491aa3bab7acd89ac40a9fc27aeed915
Right..?
Yes it is a regression, so the change there should be:
Bryant, I think you're a little confused. He's just referring to your
patch, not my later comment about an additional error case.
This is not a regression, it, and the case I mentioned in that later
note, are simply additional cases where target_setup_cmd_from_cdb
returns an error, which means that we never get far enough to call
target_execute_cmd to increment simple_cmds.
if (sense_reason != TCM_UNSUPPORTED_SCSI_OPCODE ||
sense_reason != TCM_INVALID_CDB_FIELD)
transport_complete_task_attr(cmd);
I don't think this works for a fix for the case I mentioned
(transport_check_alloc_task_attr failing). TCM_INVALID_CDB_FIELD can
get generated for a whole lot of cases, for most of which we want to
call transport_complete_task_attr.
It seems like this might be more of a design flaw to which we're just
trying to apply band-aids. I'm not sure what the right fix is, but I
don't think this is it.
So I guess since it is a regression would you have two separate patches
or just fix this in the other one?
-Bryant
--
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
--
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
