Re: [PATCH] ibmvscsis: Do not send aborted task response

2017-04-07 Thread Michael Cyr 

On 4/7/17 12:01 PM, Bryant G. Ly wrote:



On 4/7/17 11:36 AM, Bart Van Assche wrote:

On Fri, 2017-04-07 at 11:12 -0500, Bryant G. Ly wrote:
So from this stack trace it looks like the ibmvscsis driver is 
sending an

extra response through send_messages even though an abort was issued.
IBMi, reported this issue internally when they were testing the driver,
because their client didn't like getting a response back for the 
aborted op.
They are only expecting a TMR from the abort request, NOT the 
aborted op.

Hello Bryant,

What is the root cause of this behavior? Why is it that the behavior of
the ibmvscsi_tgt contradicts what has been implemented in the LIO core?
Sorry but the patch at the start of this thread looks to me like an
attempt to paper over the problem instead of addressing the root cause.

Thanks,

IBMi clients received a response for an aborted operation, so they 
sent an abort tm request.
Afterwards they get a CRQ response to the op that they aborted. That 
should not happen, because they are only supposed to get a response 
for the tm request NOT the aborted operation.
Looking at the code it looks like it is due to send messages, 
processing a response without checking to see if it was an aborted op.
This patch addresses a bug within the ibmvscsis driver and the fact 
that it SENT a response to the aborted operation(which is wrong since) 
without looking at what LIO core had done.
The driver isn't supposed to send any response to the aborted 
operation, BUT only a response to the abort tm request, which LIO core 
currently does.


-Bryant

I think I can clarify the issue here: 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.  Perhaps it would be cleaner to set some sort of 
status valid flag during queue_status instead of setting a flag in 
aborted_task?

[PATCH v2 1/6] ibmvscsis: Rearrange functions for future patches

2016-10-13 Thread Michael Cyr 
This patch reorders functions in a manner necessary for a follow-on
patch.  It also makes some minor styling changes (mostly removing extra
spaces) and fixes some typos.

There are no code changes in this patch, with one exception: due to the
reordering of the functions, I needed to explicitly declare a function
at the top of the file.  However, this will be removed in the next patch,
since the code requiring the predeclaration will be removed.

Signed-off-by: Michael Cyr <mike...@us.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 796 ---
 1 file changed, 399 insertions(+), 397 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 642b739..01a430c 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -22,7 +22,7 @@
  *
  /
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
 
 #include 
 #include 
@@ -61,6 +61,8 @@ static long ibmvscsis_parse_command(struct scsi_info *vscsi,
 
 static void ibmvscsis_adapter_idle(struct scsi_info *vscsi);
 
+static void ibmvscsis_reset_queue(struct scsi_info *vscsi, uint new_state);
+
 static void ibmvscsis_determine_resid(struct se_cmd *se_cmd,
  struct srp_rsp *rsp)
 {
@@ -81,7 +83,7 @@ static void ibmvscsis_determine_resid(struct se_cmd *se_cmd,
}
} else if (se_cmd->se_cmd_flags & SCF_OVERFLOW_BIT) {
if (se_cmd->data_direction == DMA_TO_DEVICE) {
-   /*  residual data from an overflow write */
+   /* residual data from an overflow write */
rsp->flags = SRP_RSP_FLAG_DOOVER;
rsp->data_out_res_cnt = cpu_to_be32(residual_count);
} else if (se_cmd->data_direction == DMA_FROM_DEVICE) {
@@ -101,7 +103,7 @@ static void ibmvscsis_determine_resid(struct se_cmd *se_cmd,
  * and the function returns TRUE.
  *
  * EXECUTION ENVIRONMENT:
- *  Interrupt or Process environment
+ * Interrupt or Process environment
  */
 static bool connection_broken(struct scsi_info *vscsi)
 {
@@ -324,7 +326,7 @@ static struct viosrp_crq *ibmvscsis_cmd_q_dequeue(uint mask,
 }
 
 /**
- * ibmvscsis_send_init_message() -  send initialize message to the client
+ * ibmvscsis_send_init_message() - send initialize message to the client
  * @vscsi: Pointer to our adapter structure
  * @format:Which Init Message format to send
  *
@@ -382,13 +384,13 @@ static long ibmvscsis_check_init_msg(struct scsi_info 
*vscsi, uint *format)
  vscsi->cmd_q.base_addr);
if (crq) {
*format = (uint)(crq->format);
-   rc =  ERROR;
+   rc = ERROR;
crq->valid = INVALIDATE_CMD_RESP_EL;
dma_rmb();
}
} else {
*format = (uint)(crq->format);
-   rc =  ERROR;
+   rc = ERROR;
crq->valid = INVALIDATE_CMD_RESP_EL;
dma_rmb();
}
@@ -397,166 +399,6 @@ static long ibmvscsis_check_init_msg(struct scsi_info 
*vscsi, uint *format)
 }
 
 /**
- * ibmvscsis_establish_new_q() - Establish new CRQ queue
- * @vscsi: Pointer to our adapter structure
- * @new_state: New state being established after resetting the queue
- *
- * Must be called with interrupt lock held.
- */
-static long ibmvscsis_establish_new_q(struct scsi_info *vscsi,  uint new_state)
-{
-   long rc = ADAPT_SUCCESS;
-   uint format;
-
-   vscsi->flags &= PRESERVE_FLAG_FIELDS;
-   vscsi->rsp_q_timer.timer_pops = 0;
-   vscsi->debit = 0;
-   vscsi->credit = 0;
-
-   rc = vio_enable_interrupts(vscsi->dma_dev);
-   if (rc) {
-   pr_warn("reset_queue: failed to enable interrupts, rc %ld\n",
-   rc);
-   return rc;
-   }
-
-   rc = ibmvscsis_check_init_msg(vscsi, );
-   if (rc) {
-   dev_err(>dev, "reset_queue: check_init_msg failed, rc 
%ld\n",
-   rc);
-   return rc;
-   }
-
-   if (format == UNUSED_FORMAT && new_state == WAIT_CONNECTION) {
-   rc = ibmvscsis_send_init_message(vscsi, INIT_MSG);
-   switch (rc) {
-   case H_SUCCESS:
-   case H_DROPPED:
-   case H_CLOSED:
-   rc = ADAPT_SUCCESS;
-   break;
-
-   case H_PARAMETER:
-   case H_HARDWARE:
-   break;
-
-   default:
-

[PATCH v2 2/6] ibmvscsis: Synchronize cmds at tpg_enable_store time

2016-10-13 Thread Michael Cyr 
This patch changes the way the IBM vSCSI server driver manages its
Command/Response Queue (CRQ).  We used to register the CRQ with phyp at
probe time.  Now we wait until tpg_enable_store.  Similarly, when
tpg_enable_store is called to "disable" (i.e. the stored value is 0),
we unregister the queue with phyp.

One consquence to this is that we have no need for the PART_UP_WAIT_ENAB
state, since we can't get an Init Message from the client in our CRQ if
we're waiting to be enabled, since we haven't registered the queue yet.

Signed-off-by: Michael Cyr <mike...@us.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 224 ++-
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |   2 -
 2 files changed, 38 insertions(+), 188 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 01a430c..2ce1d73 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -61,8 +61,6 @@ static long ibmvscsis_parse_command(struct scsi_info *vscsi,
 
 static void ibmvscsis_adapter_idle(struct scsi_info *vscsi);
 
-static void ibmvscsis_reset_queue(struct scsi_info *vscsi, uint new_state);
-
 static void ibmvscsis_determine_resid(struct se_cmd *se_cmd,
  struct srp_rsp *rsp)
 {
@@ -417,7 +415,6 @@ static void ibmvscsis_disconnect(struct work_struct *work)
   proc_work);
u16 new_state;
bool wait_idle = false;
-   long rc = ADAPT_SUCCESS;
 
spin_lock_bh(>intr_lock);
new_state = vscsi->new_state;
@@ -470,30 +467,12 @@ static void ibmvscsis_disconnect(struct work_struct *work)
vscsi->state = new_state;
break;
 
-   /*
-* If this is a transition into an error state.
-* a client is attempting to establish a connection
-* and has violated the RPA protocol.
-* There can be nothing pending on the adapter although
-* there can be requests in the command queue.
-*/
case WAIT_ENABLED:
-   case PART_UP_WAIT_ENAB:
switch (new_state) {
+   /* should never happen */
case ERR_DISCONNECT:
-   vscsi->flags |= RESPONSE_Q_DOWN;
-   vscsi->state = new_state;
-   vscsi->flags &= ~(SCHEDULE_DISCONNECT |
- DISCONNECT_SCHEDULED);
-   ibmvscsis_free_command_q(vscsi);
-   break;
case ERR_DISCONNECT_RECONNECT:
-   ibmvscsis_reset_queue(vscsi, WAIT_ENABLED);
-   break;
-
-   /* should never happen */
case WAIT_IDLE:
-   rc = ERROR;
dev_err(>dev, "disconnect: invalid state %d for 
WAIT_IDLE\n",
vscsi->state);
break;
@@ -630,7 +609,6 @@ static void ibmvscsis_post_disconnect(struct scsi_info 
*vscsi, uint new_state,
break;
 
case WAIT_ENABLED:
-   case PART_UP_WAIT_ENAB:
case WAIT_IDLE:
case WAIT_CONNECTION:
case CONNECTED:
@@ -675,7 +653,6 @@ static long ibmvscsis_handle_init_compl_msg(struct 
scsi_info *vscsi)
case SRP_PROCESSING:
case CONNECTED:
case WAIT_ENABLED:
-   case PART_UP_WAIT_ENAB:
default:
rc = ERROR;
dev_err(>dev, "init_msg: invalid state %d to get init 
compl msg\n",
@@ -698,10 +675,6 @@ static long ibmvscsis_handle_init_msg(struct scsi_info 
*vscsi)
long rc = ADAPT_SUCCESS;
 
switch (vscsi->state) {
-   case WAIT_ENABLED:
-   vscsi->state = PART_UP_WAIT_ENAB;
-   break;
-
case WAIT_CONNECTION:
rc = ibmvscsis_send_init_message(vscsi, INIT_COMPLETE_MSG);
switch (rc) {
@@ -737,7 +710,7 @@ static long ibmvscsis_handle_init_msg(struct scsi_info 
*vscsi)
case UNCONFIGURING:
break;
 
-   case PART_UP_WAIT_ENAB:
+   case WAIT_ENABLED:
case CONNECTED:
case SRP_PROCESSING:
case WAIT_IDLE:
@@ -800,11 +773,10 @@ static long ibmvscsis_init_msg(struct scsi_info *vscsi, 
struct viosrp_crq *crq)
 /**
  * ibmvscsis_establish_new_q() - Establish new CRQ queue
  * @vscsi: Pointer to our adapter structure
- * @new_state: New state being established after resetting the queue
  *
  * Must be called with interrupt lock held.
  */
-static long ibmvscsis_establish_new_q(struct scsi_info *vscsi, uint new_state)
+static long ibmvscsis_establish_new_q(struct scsi_info *vscsi)
 {
long rc = ADAPT_SUCCESS;
uint forma

[PATCH v2 0/6] Fixes for ibmvscsis driver

2016-10-13 Thread Michael Cyr 
Various fixes and cleanups for the ibmvscsis driver.
The first is a sort of prequel to the second, which is the primary
change (and the biggest).  The rest are fairly small fixes.

Michael Cyr (6):
  ibmvscsis: Rearrange functions for future patches
  ibmvscsis: Synchronize cmds at tpg_enable_store time
  ibmvscsis: Synchronize cmds at remove time
  ibmvscsis: Clean up properly if target_submit_cmd/tmr fails
  ibmvscsis: Return correct partition name/# to client
  ibmvscsis: Issues from Dan Carpenter/Smatch

 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 1096 +-
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |5 +-
 2 files changed, 494 insertions(+), 607 deletions(-)

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 5/6] ibmvscsis: Return correct partition name/# to client

2016-10-13 Thread Michael Cyr 
Signed-off-by: Michael Cyr <mike...@us.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index cd9f5c7..fe220a1 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3297,6 +3297,9 @@ static int ibmvscsis_probe(struct vio_dev *vdev,
strncat(vscsi->eye, vdev->name, MAX_EYE);
 
vscsi->dds.unit_id = vdev->unit_address;
+   strncpy(vscsi->dds.partition_name, partition_name,
+   sizeof(vscsi->dds.partition_name));
+   vscsi->dds.partition_num = partition_number;
 
spin_lock_bh(_dev_lock);
list_add_tail(>list, _dev_list);
@@ -3495,7 +3498,7 @@ static int ibmvscsis_get_system_info(void)
 
num = of_get_property(rootdn, "ibm,partition-no", NULL);
if (num)
-   partition_number = *num;
+   partition_number = of_read_number(num, 1);
 
of_node_put(rootdn);
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/6] ibmvscsis: Synchronize cmds at remove time

2016-10-13 Thread Michael Cyr 
This patch adds code to disconnect from the client, which will make sure
any outstanding commands have been completed, before continuing on with
the remove operation.

Signed-off-by: Michael Cyr <mike...@us.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 39 
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |  3 +++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 2ce1d73..41af435 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -469,6 +469,18 @@ static void ibmvscsis_disconnect(struct work_struct *work)
 
case WAIT_ENABLED:
switch (new_state) {
+   case UNCONFIGURING:
+   vscsi->state = new_state;
+   vscsi->flags |= RESPONSE_Q_DOWN;
+   vscsi->flags &= ~(SCHEDULE_DISCONNECT |
+ DISCONNECT_SCHEDULED);
+   dma_rmb();
+   if (vscsi->flags & CFG_SLEEPING) {
+   vscsi->flags &= ~CFG_SLEEPING;
+   complete(>unconfig);
+   }
+   break;
+
/* should never happen */
case ERR_DISCONNECT:
case ERR_DISCONNECT_RECONNECT:
@@ -481,6 +493,13 @@ static void ibmvscsis_disconnect(struct work_struct *work)
 
case WAIT_IDLE:
switch (new_state) {
+   case UNCONFIGURING:
+   vscsi->flags |= RESPONSE_Q_DOWN;
+   vscsi->state = new_state;
+   vscsi->flags &= ~(SCHEDULE_DISCONNECT |
+ DISCONNECT_SCHEDULED);
+   ibmvscsis_free_command_q(vscsi);
+   break;
case ERR_DISCONNECT:
case ERR_DISCONNECT_RECONNECT:
vscsi->state = new_state;
@@ -1186,6 +1205,15 @@ static void ibmvscsis_adapter_idle(struct scsi_info 
*vscsi)
free_qs = true;
 
switch (vscsi->state) {
+   case UNCONFIGURING:
+   ibmvscsis_free_command_q(vscsi);
+   dma_rmb();
+   isync();
+   if (vscsi->flags & CFG_SLEEPING) {
+   vscsi->flags &= ~CFG_SLEEPING;
+   complete(>unconfig);
+   }
+   break;
case ERR_DISCONNECT_RECONNECT:
ibmvscsis_reset_queue(vscsi);
pr_debug("adapter_idle, disc_rec: flags 0x%x\n", vscsi->flags);
@@ -3338,6 +3366,7 @@ static int ibmvscsis_probe(struct vio_dev *vdev,
 (unsigned long)vscsi);
 
init_completion(>wait_idle);
+   init_completion(>unconfig);
 
snprintf(wq_name, 24, "ibmvscsis%s", dev_name(>dev));
vscsi->work_q = create_workqueue(wq_name);
@@ -3393,10 +3422,11 @@ static int ibmvscsis_remove(struct vio_dev *vdev)
 
pr_debug("remove (%s)\n", dev_name(>dma_dev->dev));
 
-   /*
-* TBD: Need to handle if there are commands on the waiting_rsp q
-*  Actually, can there still be cmds outstanding to tcm?
-*/
+   spin_lock_bh(>intr_lock);
+   ibmvscsis_post_disconnect(vscsi, UNCONFIGURING, 0);
+   vscsi->flags |= CFG_SLEEPING;
+   spin_unlock_bh(>intr_lock);
+   wait_for_completion(>unconfig);
 
vio_disable_interrupts(vdev);
free_irq(vdev->irq, vscsi);
@@ -3405,7 +3435,6 @@ static int ibmvscsis_remove(struct vio_dev *vdev)
 DMA_BIDIRECTIONAL);
kfree(vscsi->map_buf);
tasklet_kill(>work_task);
-   ibmvscsis_unregister_command_q(vscsi);
ibmvscsis_destroy_command_q(vscsi);
ibmvscsis_freetimer(vscsi);
ibmvscsis_free_cmds(vscsi);
diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h
index 17e0ef4..98b0ca7 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h
@@ -257,6 +257,8 @@ struct scsi_info {
 #define SCHEDULE_DISCONNECT   0x00400
/* disconnect handler is scheduled */
 #define DISCONNECT_SCHEDULED  0x00800
+   /* remove function is sleeping */
+#define CFG_SLEEPING  0x01000
u32 flags;
/* adapter lock */
spinlock_t intr_lock;
@@ -285,6 +287,7 @@ struct scsi_info {
 
struct workqueue_struct *work_q;
struct completion wait_idle;
+   struct completion unconfig;
struct device dev;
struct vio_dev *dma_dev;
struct srp_target target;
-- 
2.5.0

--
To

[PATCH v2 6/6] ibmvscsis: Issues from Dan Carpenter/Smatch

2016-10-13 Thread Michael Cyr 
Signed-off-by: Michael Cyr <mike...@us.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index fe220a1..c9fa356 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -1896,14 +1896,7 @@ static long ibmvscsis_mad(struct scsi_info *vscsi, 
struct viosrp_crq *crq)
 
pr_debug("mad: type %d\n", be32_to_cpu(mad->type));
 
-   if (be16_to_cpu(mad->length) < 0) {
-   dev_err(>dev, "mad: length is < 0\n");
-   ibmvscsis_post_disconnect(vscsi,
- ERR_DISCONNECT_RECONNECT, 0);
-   rc = SRP_VIOLATION;
-   } else {
-   rc = ibmvscsis_process_mad(vscsi, iue);
-   }
+   rc = ibmvscsis_process_mad(vscsi, iue);
 
pr_debug("mad: status %hd, rc %ld\n", be16_to_cpu(mad->status),
 rc);
@@ -2523,7 +2516,6 @@ static void ibmvscsis_parse_cmd(struct scsi_info *vscsi,
dev_err(>dev, "0x%llx: parsing SRP descriptor table 
failed.\n",
srp->tag);
goto fail;
-   return;
}
 
cmd->rsp.sol_not = srp->sol_not;
@@ -3282,7 +3274,8 @@ static int ibmvscsis_probe(struct vio_dev *vdev,
INIT_LIST_HEAD(>waiting_rsp);
INIT_LIST_HEAD(>active_q);
 
-   snprintf(vscsi->tport.tport_name, 256, "%s", dev_name(>dev));
+   snprintf(vscsi->tport.tport_name, IBMVSCSIS_NAMELEN, "%s",
+dev_name(>dev));
 
pr_debug("probe tport_name: %s\n", vscsi->tport.tport_name);
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/6] ibmvscsis: Clean up properly if target_submit_cmd/tmr fails

2016-10-13 Thread Michael Cyr 
Signed-off-by: Michael Cyr <mike...@us.ibm.com>
Signed-off-by: Bryant G. Ly <bryan...@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 41af435..cd9f5c7 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -2560,6 +2560,10 @@ static void ibmvscsis_parse_cmd(struct scsi_info *vscsi,
   data_len, attr, dir, 0);
if (rc) {
dev_err(>dev, "target_submit_cmd failed, rc %d\n", rc);
+   spin_lock_bh(>intr_lock);
+   list_del(>list);
+   ibmvscsis_free_cmd_resources(vscsi, cmd);
+   spin_unlock_bh(>intr_lock);
goto fail;
}
return;
@@ -2639,6 +2643,9 @@ static void ibmvscsis_parse_task(struct scsi_info *vscsi,
if (rc) {
dev_err(>dev, "target_submit_tmr failed, rc 
%d\n",
rc);
+   spin_lock_bh(>intr_lock);
+   list_del(>list);
+   spin_unlock_bh(>intr_lock);
cmd->se_cmd.se_tmr_req->response =
TMR_FUNCTION_REJECTED;
}
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 3/3] Return correct partition name/number to client

2016-10-11 Thread Michael Cyr 
Signed-off-by: Michael Cyr <mike...@us.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 5c4b979..4dbbe4b 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3298,6 +3298,9 @@ static int ibmvscsis_probe(struct vio_dev *vdev,
strncat(vscsi->eye, vdev->name, MAX_EYE);
 
vscsi->dds.unit_id = vdev->unit_address;
+   strncpy(vscsi->dds.partition_name, partition_name,
+   sizeof(vscsi->dds.partition_name));
+   vscsi->dds.partition_num = partition_number;
 
spin_lock_bh(_dev_lock);
list_add_tail(>list, _dev_list);
@@ -3496,7 +3499,7 @@ static int ibmvscsis_get_system_info(void)
 
num = of_get_property(rootdn, "ibm,partition-no", NULL);
if (num)
-   partition_number = *num;
+   partition_number = of_read_number(num, 1);
 
of_node_put(rootdn);
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 2/3] Properly handle if target_submit_cmd/tmr fails

2016-10-11 Thread Michael Cyr 
Signed-off-by: Michael Cyr <mike...@us.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 34f758c..5c4b979 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -2561,6 +2561,10 @@ static void ibmvscsis_parse_cmd(struct scsi_info *vscsi,
   data_len, attr, dir, 0);
if (rc) {
dev_err(>dev, "target_submit_cmd failed, rc %d\n", rc);
+   spin_lock_bh(>intr_lock);
+   list_del(>list);
+   ibmvscsis_free_cmd_resources(vscsi, cmd);
+   spin_unlock_bh(>intr_lock);
goto fail;
}
return;
@@ -2640,6 +2644,9 @@ static void ibmvscsis_parse_task(struct scsi_info *vscsi,
if (rc) {
dev_err(>dev, "target_submit_tmr failed, rc 
%d\n",
rc);
+   spin_lock_bh(>intr_lock);
+   list_del(>list);
+   spin_unlock_bh(>intr_lock);
cmd->se_cmd.se_tmr_req->response =
TMR_FUNCTION_REJECTED;
}
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 0/3] Fixes for problems found during testing

2016-10-11 Thread Michael Cyr 
The first one seems bigger than it is, because I needed to rearrange the
order of some functions.

Michael Cyr (3):
  Synchronization of cmds during termination conditions
  Properly handle if target_submit_cmd/tmr fails
  Return correct partition name/number to client

 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 1076 ++
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |5 +-
 2 files changed, 488 insertions(+), 593 deletions(-)

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 1/3] Synchronization of cmds during termination conditions

2016-10-11 Thread Michael Cyr 
Signed-off-by: Michael Cyr <mike...@us.ibm.com>
---
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 1082 +-
 drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |5 +-
 2 files changed, 486 insertions(+), 601 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c 
b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index 642b739..34f758c 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -22,7 +22,7 @@
  *
  /
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
 
 #include 
 #include 
@@ -81,7 +81,7 @@ static void ibmvscsis_determine_resid(struct se_cmd *se_cmd,
}
} else if (se_cmd->se_cmd_flags & SCF_OVERFLOW_BIT) {
if (se_cmd->data_direction == DMA_TO_DEVICE) {
-   /*  residual data from an overflow write */
+   /* residual data from an overflow write */
rsp->flags = SRP_RSP_FLAG_DOOVER;
rsp->data_out_res_cnt = cpu_to_be32(residual_count);
} else if (se_cmd->data_direction == DMA_FROM_DEVICE) {
@@ -101,7 +101,7 @@ static void ibmvscsis_determine_resid(struct se_cmd *se_cmd,
  * and the function returns TRUE.
  *
  * EXECUTION ENVIRONMENT:
- *  Interrupt or Process environment
+ * Interrupt or Process environment
  */
 static bool connection_broken(struct scsi_info *vscsi)
 {
@@ -324,7 +324,7 @@ static struct viosrp_crq *ibmvscsis_cmd_q_dequeue(uint mask,
 }
 
 /**
- * ibmvscsis_send_init_message() -  send initialize message to the client
+ * ibmvscsis_send_init_message() - send initialize message to the client
  * @vscsi: Pointer to our adapter structure
  * @format:Which Init Message format to send
  *
@@ -382,13 +382,13 @@ static long ibmvscsis_check_init_msg(struct scsi_info 
*vscsi, uint *format)
  vscsi->cmd_q.base_addr);
if (crq) {
*format = (uint)(crq->format);
-   rc =  ERROR;
+   rc = ERROR;
crq->valid = INVALIDATE_CMD_RESP_EL;
dma_rmb();
}
} else {
*format = (uint)(crq->format);
-   rc =  ERROR;
+   rc = ERROR;
crq->valid = INVALIDATE_CMD_RESP_EL;
dma_rmb();
}
@@ -397,166 +397,6 @@ static long ibmvscsis_check_init_msg(struct scsi_info 
*vscsi, uint *format)
 }
 
 /**
- * ibmvscsis_establish_new_q() - Establish new CRQ queue
- * @vscsi: Pointer to our adapter structure
- * @new_state: New state being established after resetting the queue
- *
- * Must be called with interrupt lock held.
- */
-static long ibmvscsis_establish_new_q(struct scsi_info *vscsi,  uint new_state)
-{
-   long rc = ADAPT_SUCCESS;
-   uint format;
-
-   vscsi->flags &= PRESERVE_FLAG_FIELDS;
-   vscsi->rsp_q_timer.timer_pops = 0;
-   vscsi->debit = 0;
-   vscsi->credit = 0;
-
-   rc = vio_enable_interrupts(vscsi->dma_dev);
-   if (rc) {
-   pr_warn("reset_queue: failed to enable interrupts, rc %ld\n",
-   rc);
-   return rc;
-   }
-
-   rc = ibmvscsis_check_init_msg(vscsi, );
-   if (rc) {
-   dev_err(>dev, "reset_queue: check_init_msg failed, rc 
%ld\n",
-   rc);
-   return rc;
-   }
-
-   if (format == UNUSED_FORMAT && new_state == WAIT_CONNECTION) {
-   rc = ibmvscsis_send_init_message(vscsi, INIT_MSG);
-   switch (rc) {
-   case H_SUCCESS:
-   case H_DROPPED:
-   case H_CLOSED:
-   rc = ADAPT_SUCCESS;
-   break;
-
-   case H_PARAMETER:
-   case H_HARDWARE:
-   break;
-
-   default:
-   vscsi->state = UNDEFINED;
-   rc = H_HARDWARE;
-   break;
-   }
-   }
-
-   return rc;
-}
-
-/**
- * ibmvscsis_reset_queue() - Reset CRQ Queue
- * @vscsi: Pointer to our adapter structure
- * @new_state: New state to establish after resetting the queue
- *
- * This function calls h_free_q and then calls h_reg_q and does all
- * of the bookkeeping to get us back to where we can communicate.
- *
- * Actually, we don't always call h_free_crq.  A problem was discovered
- * where one partition would close and reopen his queue, which would
- * cause his partner to get a transport event, which would cause him to
- * close and reopen his queue, which would cause the original partition
- * to get a transport event, etc., etc.  To prevent t

[PATCH] Return TCMU-generated sense data to fabric module

2016-08-26 Thread Michael Cyr 
If an error status is passed to target_complete_cmd, then by default it
queues the command to target_complete_failure_work, which will generate
Logical Unit Communication Failure sense data, overwriting any sense data
already set in the command.  This means that any sense data returned by
TCMU does not get returned to the fabric module.

This change implements a transport_complete function for target-user which
will set the SCF_TRANSPORT_TASK_SENSE flag if we have valid sense data,
which will cause target_complete_cmd to queue the command to
target_complete_ok_work instead of target_complete_failure_work.

Signed-off-by: Michael Cyr <mike...@linux.vnet.ibm.com>
Reviewed-by: Andy Grover <agro...@redhat.com>
---
 drivers/target/target_core_user.c | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 62bf4fe..81ab42e 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -158,6 +158,14 @@ static struct genl_family tcmu_genl_family = {
.netnsok = true,
 };
 
+/* Sense Key = 2 (Not Ready)
+ * ASC/ASCQ = 0x0800 (Logical Unit Communication Failure)
+ */
+static const char lu_comm_failure_sense[18] = {
+   0x70, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x0a,
+   0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00,
+   0x00, 0x00 };
+
 static struct tcmu_cmd *tcmu_alloc_cmd(struct se_cmd *se_cmd)
 {
struct se_device *se_dev = se_cmd->se_dev;
@@ -574,9 +582,11 @@ static void tcmu_handle_completion(struct tcmu_cmd *cmd, 
struct tcmu_cmd_entry *
pr_warn("TCMU: Userspace set UNKNOWN_OP flag on se_cmd %p\n",
cmd->se_cmd);
entry->rsp.scsi_status = SAM_STAT_CHECK_CONDITION;
+   memcpy(se_cmd->sense_buffer, lu_comm_failure_sense,
+  sizeof(lu_comm_failure_sense));
} else if (entry->rsp.scsi_status == SAM_STAT_CHECK_CONDITION) {
memcpy(se_cmd->sense_buffer, entry->rsp.sense_buffer,
-  se_cmd->scsi_sense_length);
+  TRANSPORT_SENSE_BUFFER);
free_data_area(udev, cmd);
} else if (se_cmd->se_cmd_flags & SCF_BIDI) {
DECLARE_BITMAP(bitmap, DATA_BLOCK_BITS);
@@ -679,6 +689,8 @@ static int tcmu_check_expired_cmd(int id, void *p, void 
*data)
return 0;
 
set_bit(TCMU_CMD_BIT_EXPIRED, >flags);
+   memcpy(cmd->se_cmd->sense_buffer, lu_comm_failure_sense,
+  sizeof(lu_comm_failure_sense));
target_complete_cmd(cmd->se_cmd, SAM_STAT_CHECK_CONDITION);
cmd->se_cmd = NULL;
 
@@ -1145,6 +1157,17 @@ tcmu_parse_cdb(struct se_cmd *cmd)
return passthrough_parse_cdb(cmd, tcmu_pass_op);
 }
 
+static void tcmu_transport_complete(struct se_cmd *cmd, struct scatterlist *sg,
+   unsigned char *sense_buffer)
+{
+   if (cmd->scsi_status == SAM_STAT_CHECK_CONDITION)
+   /* Setting this flag will prevent target_complete_cmd from
+* calling target_complete_failure_work, which would overwrite
+* the sense data we already set.
+*/
+   cmd->se_cmd_flags |= SCF_TRANSPORT_TASK_SENSE;
+}
+
 static const struct target_backend_ops tcmu_ops = {
.name   = "user",
.owner  = THIS_MODULE,
@@ -1155,6 +1178,7 @@ static const struct target_backend_ops tcmu_ops = {
.configure_device   = tcmu_configure_device,
.free_device= tcmu_free_device,
.parse_cdb  = tcmu_parse_cdb,
+   .transport_complete = tcmu_transport_complete,
.set_configfs_dev_params = tcmu_set_configfs_dev_params,
.show_configfs_dev_params = tcmu_show_configfs_dev_params,
.get_device_type= sbc_get_device_type,
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7] ibmvscsis: Initial commit of IBM VSCSI Tgt Driver

2016-06-23 Thread Michael Cyr 

Responding to those issues not already addressed by Bryant.

On 6/23/16 9:22 AM, Christoph Hellwig wrote:



+   vscsi->flags &= (~PROCESSING_MAD);

No need for the braces here. (ditto for many other places later on)

Fixed.



+static struct ibmvscsis_tport *ibmvscsis_lookup_port(const char *name)
+{
+   struct ibmvscsis_tport *tport = NULL;
+   struct vio_dev *vdev;
+   struct scsi_info *vscsi;
+
+   spin_lock_bh(_dev_lock);
+   list_for_each_entry(vscsi, _dev_list, list) {
+   vdev = vscsi->dma_dev;
+   if (!strcmp(dev_name(>dev), name)) {
+   tport = >tport;
+   break;
+   }
+   }
+   spin_unlock_bh(_dev_lock);

Without grabbing a reference this looks inherently unsafe.

I don't understand your concern.  Could you please provide more detail?

+static void ibmvscsis_scheduler(struct work_struct *work)

Odd name for a work item.

Not sure why it seems odd.  It schedules work to TCM.

+{
+   struct ibmvscsis_cmd *cmd = container_of(work, struct ibmvscsis_cmd,
+work);
+   struct scsi_info *vscsi = cmd->adapter;
+
+   spin_lock_bh(>intr_lock);
+
+   /* Remove from schedule_q */
+   list_del(>list);

What do you need the schedule_q for as the workqueue already tracks
the commands?
There are a number of places where we need to see if there is anything 
on the work queue, and I am not aware of an existing function to do 
this, so we keep our own list of commands which have been queued.




+static void ibmvscsis_alloc_common_locks(struct scsi_info *vscsi)
+{
+   spin_lock_init(>intr_lock);
+}
+
+static void ibmvscsis_free_common_locks(struct scsi_info *vscsi)
+{
+   /* Nothing to do here */
+}

No need for these wrapers.

Removed.

+static irqreturn_t ibmvscsis_interrupt(int dummy, void *data)
+{
+   struct scsi_info *vscsi = data;
+
+   vio_disable_interrupts(vscsi->dma_dev);
+   tasklet_schedule(>work_task);
+
+   return IRQ_HANDLED;
+}

Can you explain the need for the tasklet?  There shouldn't be a whole
lot of working before passing the command off to the workqueue anyway.
There are some requests, like SRP Login and the various MADs, which can 
be handled at the interrupt level, and so we do so.


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fix for hang of Ordered task in TCM

2016-05-18 Thread Michael Cyr 


On 5/18/16 12:53 AM, Nicholas A. Bellinger wrote:

Hi Michael,

On Fri, 2016-05-13 at 17:15 -0500, Michael Cyr wrote:

If a command with a Simple task attribute is failed due to a Unit
Attention, then a subsequent command with an Ordered task attribute will
hang forever.  The reason for this is that the Unit Attention status is
checked for in target_setup_cmd_from_cdb, before the call to
target_execute_cmd, which calls target_handle_task_attr, which in turn
increments dev->simple_cmds.  However, transport_generic_request_failure
still calls transport_complete_task_attr, which will decrement
dev->simple_cmds.  In this case, simple_cmds is now -1.  So when a
command with the Ordered task attribute is sent, target_handle_task_attr
sees that dev->simple_cmds is not 0, so it decides it can't execute the
command until all the (nonexistent) Simple commands have completed.


Thanks for reporting this bug.  Comments below.


The solution I've implemented is to move target_scsi3_ua_check, as well as
target_alua_state_check and target_check_reservation, into
target_execute_cmd, after the call to target_handle_task_attr.  I believe
this is actually the correct way this should be handled.  According to
SAM-4 r14, under section 5.14:

"h) if a command other than INQUIRY, REPORT LUNS, REQUEST SENSE, or NOTIFY
DATA TRANSFER DEVICE enters the enabled command state while a unit
attention condition exists for the SCSI initiator port associated with
the I_T nexus on which the command was received, the device server shall
terminate the command with a CHECK CONDITION status. The device server
shall provide sense data that reports a unit attention condition for the
SCSI initiator port that sent the command on the I_T nexus."

But according to section 8.5 and 8.6, a command which is not yet executed
because of the presence of other tasks in the task set (i.e., one for
which target_handle_task_attr returns true) would not enter the enabled
command state; it would be in the dormant command state.
target_execute_cmd would get called when a command entered the enabled
command state, and thus that is the appropriate place to check for Unit
Attenion.  Similarly, though not quite as explicit, section 5.3.3 tells
us that a Reservation Conflict status has a lower precedence than a Unit
Attention, and so this would also seem to be the appropriate place to
call target_check_reservation.  I'm less sure about
target_alua_state_check, since I'm not very familiar with ALUA.

Signed-off-by: Michael Cyr <mike...@linux.vnet.ibm.com>
---
  drivers/target/target_core_transport.c | 41 --
  1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 6c089af..2ee5502 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1303,23 +1303,6 @@ target_setup_cmd_from_cdb(struct se_cmd *cmd, unsigned 
char *cdb)
  
  	trace_target_sequencer_start(cmd);
  
-	/*

-* Check for an existing UNIT ATTENTION condition
-*/
-   ret = target_scsi3_ua_check(cmd);
-   if (ret)
-   return ret;
-
-   ret = target_alua_state_check(cmd);
-   if (ret)
-   return ret;
-
-   ret = target_check_reservation(cmd);
-   if (ret) {
-   cmd->scsi_status = SAM_STAT_RESERVATION_CONFLICT;
-   return ret;
-   }
-
ret = dev->transport->parse_cdb(cmd);
if (ret == TCM_UNSUPPORTED_SCSI_OPCODE)
pr_warn_ratelimited("%s/%s: Unsupported SCSI Opcode 0x%02x, sending 
CHECK_CONDITION.\n",
@@ -1865,6 +1848,8 @@ static int __transport_check_aborted_status(struct se_cmd 
*, int);
  
  void target_execute_cmd(struct se_cmd *cmd)

  {
+   sense_reason_t ret;
+
/*
 * Determine if frontend context caller is requesting the stopping of
 * this command for frontend exceptions.
@@ -1899,6 +1884,28 @@ void target_execute_cmd(struct se_cmd *cmd)
return;
}
  
+	/*

+* Check for an existing UNIT ATTENTION condition
+*/
+   ret = target_scsi3_ua_check(cmd);
+   if (ret) {
+   transport_generic_request_failure(cmd, ret);
+   return;
+   }
+
+   ret = target_alua_state_check(cmd);
+   if (ret) {
+   transport_generic_request_failure(cmd, ret);
+   return;
+   }
+
+   ret = target_check_reservation(cmd);
+   if (ret) {
+   cmd->scsi_status = SAM_STAT_RESERVATION_CONFLICT;
+   transport_generic_request_failure(cmd, ret);
+   return;
+   }
+
__target_execute_cmd(cmd);
  }
  EXPORT_SYMBOL(target_execute_cmd);

So AFAICT for delayed commands, the above patch ends up skipping these
three checks subsequently when doing __target_execute_cmd() directly
from target_restart_delayed_cmds(), no..?

Af

[PATCH] Fix for hang of Ordered task in TCM

2016-05-13 Thread Michael Cyr 
If a command with a Simple task attribute is failed due to a Unit
Attention, then a subsequent command with an Ordered task attribute will
hang forever.  The reason for this is that the Unit Attention status is
checked for in target_setup_cmd_from_cdb, before the call to
target_execute_cmd, which calls target_handle_task_attr, which in turn
increments dev->simple_cmds.  However, transport_generic_request_failure
still calls transport_complete_task_attr, which will decrement
dev->simple_cmds.  In this case, simple_cmds is now -1.  So when a
command with the Ordered task attribute is sent, target_handle_task_attr
sees that dev->simple_cmds is not 0, so it decides it can't execute the
command until all the (nonexistent) Simple commands have completed.

The solution I've implemented is to move target_scsi3_ua_check, as well as
target_alua_state_check and target_check_reservation, into
target_execute_cmd, after the call to target_handle_task_attr.  I believe
this is actually the correct way this should be handled.  According to
SAM-4 r14, under section 5.14:

"h) if a command other than INQUIRY, REPORT LUNS, REQUEST SENSE, or NOTIFY
DATA TRANSFER DEVICE enters the enabled command state while a unit
attention condition exists for the SCSI initiator port associated with
the I_T nexus on which the command was received, the device server shall
terminate the command with a CHECK CONDITION status. The device server
shall provide sense data that reports a unit attention condition for the
SCSI initiator port that sent the command on the I_T nexus."

But according to section 8.5 and 8.6, a command which is not yet executed
because of the presence of other tasks in the task set (i.e., one for
which target_handle_task_attr returns true) would not enter the enabled
command state; it would be in the dormant command state.
target_execute_cmd would get called when a command entered the enabled
command state, and thus that is the appropriate place to check for Unit
Attenion.  Similarly, though not quite as explicit, section 5.3.3 tells
us that a Reservation Conflict status has a lower precedence than a Unit
Attention, and so this would also seem to be the appropriate place to
call target_check_reservation.  I'm less sure about
target_alua_state_check, since I'm not very familiar with ALUA.

Signed-off-by: Michael Cyr <mike...@linux.vnet.ibm.com>
---
 drivers/target/target_core_transport.c | 41 --
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 6c089af..2ee5502 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1303,23 +1303,6 @@ target_setup_cmd_from_cdb(struct se_cmd *cmd, unsigned 
char *cdb)
 
trace_target_sequencer_start(cmd);
 
-   /*
-* Check for an existing UNIT ATTENTION condition
-*/
-   ret = target_scsi3_ua_check(cmd);
-   if (ret)
-   return ret;
-
-   ret = target_alua_state_check(cmd);
-   if (ret)
-   return ret;
-
-   ret = target_check_reservation(cmd);
-   if (ret) {
-   cmd->scsi_status = SAM_STAT_RESERVATION_CONFLICT;
-   return ret;
-   }
-
ret = dev->transport->parse_cdb(cmd);
if (ret == TCM_UNSUPPORTED_SCSI_OPCODE)
pr_warn_ratelimited("%s/%s: Unsupported SCSI Opcode 0x%02x, 
sending CHECK_CONDITION.\n",
@@ -1865,6 +1848,8 @@ static int __transport_check_aborted_status(struct se_cmd 
*, int);
 
 void target_execute_cmd(struct se_cmd *cmd)
 {
+   sense_reason_t ret;
+
/*
 * Determine if frontend context caller is requesting the stopping of
 * this command for frontend exceptions.
@@ -1899,6 +1884,28 @@ void target_execute_cmd(struct se_cmd *cmd)
return;
}
 
+   /*
+* Check for an existing UNIT ATTENTION condition
+*/
+   ret = target_scsi3_ua_check(cmd);
+   if (ret) {
+   transport_generic_request_failure(cmd, ret);
+   return;
+   }
+
+   ret = target_alua_state_check(cmd);
+   if (ret) {
+   transport_generic_request_failure(cmd, ret);
+   return;
+   }
+
+   ret = target_check_reservation(cmd);
+   if (ret) {
+   cmd->scsi_status = SAM_STAT_RESERVATION_CONFLICT;
+   transport_generic_request_failure(cmd, ret);
+   return;
+   }
+
__target_execute_cmd(cmd);
 }
 EXPORT_SYMBOL(target_execute_cmd);
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html