Re: [PATCH] ibmvfc: Fixup duplicate response detection

[Tyrel Datwyler <tyreld@linux.ibm.com>]

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);
>  	}
>