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

[Bryant G. Ly]

On 10/11/16 5:58 PM, Michael Cyr wrote:

Signed-off-by: Michael Cyr 
---
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 1082 +-
  drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.h |5 +-
  2 files changed, 486 insertions(+), 601 deletions(-)


I would make the first patch reorganization and styling fixes, no actual 
code changes.
Then for each of the commits make sure you amend them to give more 
description

as to what you are doing in each specific patch.

i.e. First patch:

ibmvscsis: Re-organization of commands/styling

Need to move functions around in order for the proceeding patches to work.

Patch 2/3 and 3/3 look fine to me after you change the title to include 
"ibmvscsis:" in it.


-Bryant

--
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 v1 1/3] Synchronization of cmds during termination conditions

[Greg KH]

On Tue, Oct 11, 2016 at 05:58:03PM -0500, Michael Cyr wrote:
> Signed-off-by: Michael Cyr 

I almost never accept patches with no changelog information.  For a
patch that changes 1082 lines in a non-trivial way, you would think that
you could explain why you are doing this work...

Also, check the prefix of your subject, it give no clue as to where in
the kernel your patches touch :(

thanks,

greg k-h
--
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

[Michael Cyr]

Signed-off-by: Michael Cyr 
---
 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 this, we don't
- * actually close our queue if the client initiated the reset, (i.e.
- * either we got a transport