[PATCH v2] fixup "qla2xxx: Optimize NPIV tear down process"

[PATCH v2] fixup "qla2xxx: Optimize NPIV tear down process"

[Martin Wilck]

From: Martin Wilck <mwilck@suse.com>

Hello Martin,

this patch fixes two issues in patch 02/14 in Himanshu's latest
qla2xxx series ("qla2xxx: Bug fixes for the driver") from
Sept. 12th, which you applied onto 5.4/scsi-fixes already.
See https://marc.info/?l=linux-scsi&m=156951704106671&w=2

I'm assuming that Himanshu and Quinn are working on another
series of fixes, in which case that should take precedence
over this patch. I just wanted to provide this so that the
already known problems are fixed in your tree.

v2: check loop condition only once (Bart van Assche)

Commit message follows:

Fix two issues with the previously submitted patch
"qla2xxx: Optimize NPIV tear down process": a missing negation
in a wait_event_timeout() condition, and a missing loop end
condition.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Fixes: f5187b7d1ac6 ("scsi: qla2xxx: Optimize NPIV tear down process")
---
 drivers/scsi/qla2xxx/qla_mid.c | 8 +++++---
 drivers/scsi/qla2xxx/qla_os.c  | 8 +++++---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
index 6afad68e5ba2..238240984bc1 100644
--- a/drivers/scsi/qla2xxx/qla_mid.c
+++ b/drivers/scsi/qla2xxx/qla_mid.c
@@ -76,9 +76,11 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha)
 	 * ensures no active vp_list traversal while the vport is removed
 	 * from the queue)
 	 */
-	for (i = 0; i < 10 && atomic_read(&vha->vref_count); i++)
-		wait_event_timeout(vha->vref_waitq,
-		    atomic_read(&vha->vref_count), HZ);
+	for (i = 0; i < 10; i++) {
+		if (wait_event_timeout(vha->vref_waitq,
+		    !atomic_read(&vha->vref_count), HZ) > 0)
+			break;
+	}
 
 	spin_lock_irqsave(&ha->vport_slock, flags);
 	if (atomic_read(&vha->vref_count)) {
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 6e627e521562..ee5b6cba9872 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1119,9 +1119,11 @@ qla2x00_wait_for_sess_deletion(scsi_qla_host_t *vha)
 
 	qla2x00_mark_all_devices_lost(vha, 0);
 
-	for (i = 0; i < 10; i++)
-		wait_event_timeout(vha->fcport_waitQ, test_fcport_count(vha),
-		    HZ);
+	for (i = 0; i < 10; i++) {
+		if (wait_event_timeout(vha->fcport_waitQ,
+		    test_fcport_count(vha), HZ) > 0)
+			break;
+	}
 
 	flush_workqueue(vha->hw->wq);
 }
-- 
2.12.3

Re: [PATCH v2] fixup "qla2xxx: Optimize NPIV tear down process"

[Martin K. Petersen]

> this patch fixes two issues in patch 02/14 in Himanshu's latest
> qla2xxx series ("qla2xxx: Bug fixes for the driver") from Sept. 12th,
> which you applied onto 5.4/scsi-fixes already.  See
> https://marc.info/?l=linux-scsi&m=156951704106671&w=2
>
> I'm assuming that Himanshu and Quinn are working on another
> series of fixes, in which case that should take precedence
> over this patch. I just wanted to provide this so that the
> already known problems are fixed in your tree.

Himanshu: Please review. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2] fixup "qla2xxx: Optimize NPIV tear down process"

[Bart Van Assche]

On 2019-10-02 08:41, Martin Wilck wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> Hello Martin,
> 
> this patch fixes two issues in patch 02/14 in Himanshu's latest
> qla2xxx series ("qla2xxx: Bug fixes for the driver") from
> Sept. 12th, which you applied onto 5.4/scsi-fixes already.
> See https://marc.info/?l=linux-scsi&m=156951704106671&w=2
> 
> I'm assuming that Himanshu and Quinn are working on another
> series of fixes, in which case that should take precedence
> over this patch. I just wanted to provide this so that the
> already known problems are fixed in your tree.
> 
> v2: check loop condition only once (Bart van Assche)
> 
> Commit message follows:
> 
> Fix two issues with the previously submitted patch
> "qla2xxx: Optimize NPIV tear down process": a missing negation
> in a wait_event_timeout() condition, and a missing loop end
> condition.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> Fixes: f5187b7d1ac6 ("scsi: qla2xxx: Optimize NPIV tear down process")
> ---
>  drivers/scsi/qla2xxx/qla_mid.c | 8 +++++---
>  drivers/scsi/qla2xxx/qla_os.c  | 8 +++++---
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_mid.c b/drivers/scsi/qla2xxx/qla_mid.c
> index 6afad68e5ba2..238240984bc1 100644
> --- a/drivers/scsi/qla2xxx/qla_mid.c
> +++ b/drivers/scsi/qla2xxx/qla_mid.c
> @@ -76,9 +76,11 @@ qla24xx_deallocate_vp_id(scsi_qla_host_t *vha)
>  	 * ensures no active vp_list traversal while the vport is removed
>  	 * from the queue)
>  	 */
> -	for (i = 0; i < 10 && atomic_read(&vha->vref_count); i++)
> -		wait_event_timeout(vha->vref_waitq,
> -		    atomic_read(&vha->vref_count), HZ);
> +	for (i = 0; i < 10; i++) {
> +		if (wait_event_timeout(vha->vref_waitq,
> +		    !atomic_read(&vha->vref_count), HZ) > 0)
> +			break;
> +	}
>  
>  	spin_lock_irqsave(&ha->vport_slock, flags);
>  	if (atomic_read(&vha->vref_count)) {
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 6e627e521562..ee5b6cba9872 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -1119,9 +1119,11 @@ qla2x00_wait_for_sess_deletion(scsi_qla_host_t *vha)
>  
>  	qla2x00_mark_all_devices_lost(vha, 0);
>  
> -	for (i = 0; i < 10; i++)
> -		wait_event_timeout(vha->fcport_waitQ, test_fcport_count(vha),
> -		    HZ);
> +	for (i = 0; i < 10; i++) {
> +		if (wait_event_timeout(vha->fcport_waitQ,
> +		    test_fcport_count(vha), HZ) > 0)
> +			break;
> +	}
>  
>  	flush_workqueue(vha->hw->wq);
>  }

This patch looks fine to me although I'm still wondering what the
difference is between a loop with wait_event_timeout() calls and a
single wait_event_timeout() call with a longer timeout?

Thanks,

Bart.

Re: [PATCH v2] fixup "qla2xxx: Optimize NPIV tear down process"

[Martin Wilck]

Martin, Himanshu,

On Thu, 2019-10-03 at 21:57 -0400,  Martin K. Petersen wrote:
> > this patch fixes two issues in patch 02/14 in Himanshu's latest
> > qla2xxx series ("qla2xxx: Bug fixes for the driver") from Sept.
> > 12th,
> > which you applied onto 5.4/scsi-fixes already.  See
> > https://marc.info/?l=linux-scsi&m=156951704106671&w=2
> > 
> > I'm assuming that Himanshu and Quinn are working on another
> > series of fixes, in which case that should take precedence
> > over this patch. I just wanted to provide this so that the
> > already known problems are fixed in your tree.
> 
> Himanshu: Please review. Thanks!
> 

this patch is still not merged since a month although Quinn had
basically ack'd the propsed change 
(https://marc.info/?l=linux-scsi&m=156951704106671&w=2).

Do you want me to resubmit, and if yes, do you request changes?

Regards,
Martin Wilck