[09/17] target/core: Always call transport_complete_callback() upon failure
diff mbox series

Message ID 20180917213554.987-10-bvanassche@acm.org
State New, archived
Headers show
Series
  • Make ABORT and LUN RESET handling synchronous
Related show

Commit Message

Bart Van Assche Sept. 17, 2018, 9:35 p.m. UTC
Call transport_complete_callback() not only if COMPARE AND WRITE fails but
also if XDWRITEREAD fails. This makes the code more systematic.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Nicholas Bellinger <nab@linux-iscsi.org>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_sbc.c       |  6 +++++-
 drivers/target/target_core_transport.c | 11 +++--------
 2 files changed, 8 insertions(+), 9 deletions(-)

Comments

Christoph Hellwig Oct. 6, 2018, 12:13 p.m. UTC | #1
On Mon, Sep 17, 2018 at 02:35:46PM -0700, Bart Van Assche wrote:
> Call transport_complete_callback() not only if COMPARE AND WRITE fails but
> also if XDWRITEREAD fails. This makes the code more systematic.

This looks ok:

Reviewed-by: Christoph Hellwig <hch@lst.de>

But I wonder if transport_complete_callback should really be split into
multiple more specific callbacks instead.
Nicholas A. Bellinger Oct. 7, 2018, 1:34 a.m. UTC | #2
On Mon, 2018-09-17 at 14:35 -0700, Bart Van Assche wrote:
> Call transport_complete_callback() not only if COMPARE AND WRITE fails but
> also if XDWRITEREAD fails. This makes the code more systematic.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> Cc: Nicholas Bellinger <nab@linux-iscsi.org>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/target/target_core_sbc.c       |  6 +++++-
>  drivers/target/target_core_transport.c | 11 +++--------
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
> index 4719e6a98430..c155e12ae8b9 100644
> --- a/drivers/target/target_core_sbc.c
> +++ b/drivers/target/target_core_sbc.c
> @@ -360,6 +360,10 @@ static sense_reason_t xdreadwrite_callback(struct se_cmd *cmd, bool success,
>  	unsigned int offset;
>  	sense_reason_t ret = TCM_NO_SENSE;
>  	int i, count;
> +
> +	if (!success)
> +		return 0;
> +
>  	/*
>  	 * From sbc3r22.pdf section 5.48 XDWRITEREAD (10) command
>  	 *
> @@ -431,7 +435,7 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success,
>  	 * sent to the backend driver.
>  	 */
>  	spin_lock_irq(&cmd->t_state_lock);
> -	if (cmd->transport_state & CMD_T_SENT) {
> +	if (success) {
>  		cmd->se_cmd_flags |= SCF_COMPARE_AND_WRITE_POST;
>  		*post_ret = 1;
>  
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index ec3cb16b9e0e..79fa79afcdc2 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -1793,7 +1793,7 @@ EXPORT_SYMBOL(target_submit_tmr);
>  void transport_generic_request_failure(struct se_cmd *cmd,
>  		sense_reason_t sense_reason)
>  {
> -	int ret = 0, post_ret = 0;
> +	int ret = 0;
>  
>  	pr_debug("-----[ Storage Engine Exception; sense_reason %d\n",
>  		 sense_reason);
> @@ -1804,13 +1804,8 @@ void transport_generic_request_failure(struct se_cmd *cmd,
>  	 */
>  	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.
> -	 */
> -	if ((cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE) &&
> -	     cmd->transport_complete_callback)
> -		cmd->transport_complete_callback(cmd, false, &post_ret);
> +	if (cmd->transport_complete_callback)
> +		cmd->transport_complete_callback(cmd, false, NULL);
>  
>  	if (transport_check_aborted_status(cmd, 1))
>  		return;

Looks OK wrt earlier COMPARE_AND_WRITE failure cases originally
addressed in commit 9b2792c3d.

Reviewed-by: Nicholas Bellinger <nab@linux-iscsi.org>

Patch
diff mbox series

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 4719e6a98430..c155e12ae8b9 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -360,6 +360,10 @@  static sense_reason_t xdreadwrite_callback(struct se_cmd *cmd, bool success,
 	unsigned int offset;
 	sense_reason_t ret = TCM_NO_SENSE;
 	int i, count;
+
+	if (!success)
+		return 0;
+
 	/*
 	 * From sbc3r22.pdf section 5.48 XDWRITEREAD (10) command
 	 *
@@ -431,7 +435,7 @@  static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success,
 	 * sent to the backend driver.
 	 */
 	spin_lock_irq(&cmd->t_state_lock);
-	if (cmd->transport_state & CMD_T_SENT) {
+	if (success) {
 		cmd->se_cmd_flags |= SCF_COMPARE_AND_WRITE_POST;
 		*post_ret = 1;
 
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index ec3cb16b9e0e..79fa79afcdc2 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1793,7 +1793,7 @@  EXPORT_SYMBOL(target_submit_tmr);
 void transport_generic_request_failure(struct se_cmd *cmd,
 		sense_reason_t sense_reason)
 {
-	int ret = 0, post_ret = 0;
+	int ret = 0;
 
 	pr_debug("-----[ Storage Engine Exception; sense_reason %d\n",
 		 sense_reason);
@@ -1804,13 +1804,8 @@  void transport_generic_request_failure(struct se_cmd *cmd,
 	 */
 	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.
-	 */
-	if ((cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE) &&
-	     cmd->transport_complete_callback)
-		cmd->transport_complete_callback(cmd, false, &post_ret);
+	if (cmd->transport_complete_callback)
+		cmd->transport_complete_callback(cmd, false, NULL);
 
 	if (transport_check_aborted_status(cmd, 1))
 		return;