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@xxxxxxxxxxxxxxx
> Signed-off-by: Brian King <brking@xxxxxxxxxxxxxxxxxx>
Acked-by: Tyrel Datwyler <tyreld@xxxxxxxxxxxxx>
> ---
> 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);
> }
>
