diff mbox series

ibmvfc: Fixup duplicate response detection

Message ID 20211019152129.16558-1-brking@linux.vnet.ibm.com (mailing list archive)
State Accepted
Headers show
Series ibmvfc: Fixup duplicate response detection | expand

Commit Message

Brian King Oct. 19, 2021, 3:21 p.m. UTC
Commit a264cf5e81c7 ("scsi: ibmvfc: Fix command state accounting and stale response detection")
introduced a regression in detecting duplicate responses. This was observed
in test where a command was sent to the VIOS and completed before
ibmvfc_send_event set the active flag to 1, which resulted in the
atomic_dec_if_positive call in ibmvfc_handle_crq thinking this was a
duplicate response, which resulted in scsi_done not getting called, so we
then hit a scsi command timeout for this command once the timeout expires.
This simply ensures the active flag gets set prior to making the hcall to
send the command to the VIOS, in order to close this window.

Fixes: a264cf5e81c7 ("scsi: ibmvfc: Fix command state accounting and stale response detection")
Cc: stable@vger.kernel.org
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Tyrel Datwyler Oct. 19, 2021, 6:15 p.m. UTC | #1
On 10/19/21 8:21 AM, Brian King wrote:
> Commit a264cf5e81c7 ("scsi: ibmvfc: Fix command state accounting and stale response detection")
> introduced a regression in detecting duplicate responses. This was observed
> in test where a command was sent to the VIOS and completed before
> ibmvfc_send_event set the active flag to 1, which resulted in the
> atomic_dec_if_positive call in ibmvfc_handle_crq thinking this was a
> duplicate response, which resulted in scsi_done not getting called, so we
> then hit a scsi command timeout for this command once the timeout expires.
> This simply ensures the active flag gets set prior to making the hcall to
> send the command to the VIOS, in order to close this window.
> 
> Fixes: a264cf5e81c7 ("scsi: ibmvfc: Fix command state accounting and stale response detection")
> Cc: stable@vger.kernel.org
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>

Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com>

> ---
>  drivers/scsi/ibmvscsi/ibmvfc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index a4b0a12f8a97..d0eab5700dc5 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -1696,6 +1696,7 @@ static int ibmvfc_send_event(struct ibmvfc_event *evt,
>  
>  	spin_lock_irqsave(&evt->queue->l_lock, flags);
>  	list_add_tail(&evt->queue_list, &evt->queue->sent);
> +	atomic_set(&evt->active, 1);
>  
>  	mb();
>  
> @@ -1710,6 +1711,7 @@ static int ibmvfc_send_event(struct ibmvfc_event *evt,
>  				     be64_to_cpu(crq_as_u64[1]));
>  
>  	if (rc) {
> +		atomic_set(&evt->active, 0);
>  		list_del(&evt->queue_list);
>  		spin_unlock_irqrestore(&evt->queue->l_lock, flags);
>  		del_timer(&evt->timer);
> @@ -1737,7 +1739,6 @@ static int ibmvfc_send_event(struct ibmvfc_event *evt,
>  
>  		evt->done(evt);
>  	} else {
> -		atomic_set(&evt->active, 1);
>  		spin_unlock_irqrestore(&evt->queue->l_lock, flags);
>  		ibmvfc_trc_start(evt);
>  	}
>
Martin K. Petersen Oct. 21, 2021, 3:40 a.m. UTC | #2
On Tue, 19 Oct 2021 10:21:29 -0500, Brian King wrote:

> Commit a264cf5e81c7 ("scsi: ibmvfc: Fix command state accounting and stale response detection")
> introduced a regression in detecting duplicate responses. This was observed
> in test where a command was sent to the VIOS and completed before
> ibmvfc_send_event set the active flag to 1, which resulted in the
> atomic_dec_if_positive call in ibmvfc_handle_crq thinking this was a
> duplicate response, which resulted in scsi_done not getting called, so we
> then hit a scsi command timeout for this command once the timeout expires.
> This simply ensures the active flag gets set prior to making the hcall to
> send the command to the VIOS, in order to close this window.
> 
> [...]

Applied to 5.15/scsi-fixes, thanks!

[1/1] ibmvfc: Fixup duplicate response detection
      https://git.kernel.org/mkp/scsi/c/e20f80b9b163
diff mbox series

Patch

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index a4b0a12f8a97..d0eab5700dc5 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -1696,6 +1696,7 @@  static int ibmvfc_send_event(struct ibmvfc_event *evt,
 
 	spin_lock_irqsave(&evt->queue->l_lock, flags);
 	list_add_tail(&evt->queue_list, &evt->queue->sent);
+	atomic_set(&evt->active, 1);
 
 	mb();
 
@@ -1710,6 +1711,7 @@  static int ibmvfc_send_event(struct ibmvfc_event *evt,
 				     be64_to_cpu(crq_as_u64[1]));
 
 	if (rc) {
+		atomic_set(&evt->active, 0);
 		list_del(&evt->queue_list);
 		spin_unlock_irqrestore(&evt->queue->l_lock, flags);
 		del_timer(&evt->timer);
@@ -1737,7 +1739,6 @@  static int ibmvfc_send_event(struct ibmvfc_event *evt,
 
 		evt->done(evt);
 	} else {
-		atomic_set(&evt->active, 1);
 		spin_unlock_irqrestore(&evt->queue->l_lock, flags);
 		ibmvfc_trc_start(evt);
 	}