Re: [PATCH] bnx2fc: timeout calculation invalid for bnx2fc_eh_abort()

From: cdupuis1@gmail.com
To: Laurence Oberman <loberman@redhat.com>,
	QLogic-Storage-Upstream@qlogic.com, linux-scsi@vger.kernel.org,
	djeffery@redhat.com, jpittman@redhat.com
Subject: Re: [PATCH] bnx2fc: timeout calculation invalid for bnx2fc_eh_abort()
Date: Tue, 19 Nov 2019 22:14:32 -0500	[thread overview]
Message-ID: <4542ba273577056dbe530714f43af29ce5258814.camel@gmail.com> (raw)
In-Reply-To: <1574178394-16635-1-git-send-email-loberman@redhat.com>

Looks like the original calculation assumed that rp->r_a_tov was
specified in seconds hence the HZ multiplier.  Converting using
msecs_to_jiffies() would seem to be the correct way to do it.

Reviewed-by: Chad Dupuis <cdupuis1@gmail.com>

On Tue, 2019-11-19 at 10:46 -0500, Laurence Oberman wrote:
> In the bnx2fc_eh_abort() function there is a calculation for
> wait_for_completion that uses a HZ multiplier.
> This is incorrect, it scales the timeout by 1000 seconds
> instead of converting the ms value to jiffies.
> Therefore change the calculation.
> 
> Reported-by: David Jeffery <djeffery@redhat.com>
> Reviewed-by: John Pittman .jpittman@redhat.com>
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> ---
>  drivers/scsi/bnx2fc/bnx2fc_io.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
> b/drivers/scsi/bnx2fc/bnx2fc_io.c
> index 401743e..d8ae6d0 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> @@ -1242,7 +1242,7 @@ int bnx2fc_eh_abort(struct scsi_cmnd *sc_cmd)
>  
>  	/* Wait 2 * RA_TOV + 1 to be sure timeout function hasn't fired
> */
>  	time_left = wait_for_completion_timeout(&io_req->abts_done,
> -						(2 * rp->r_a_tov + 1) *
> HZ);
> +					msecs_to_jiffies(2 * rp-
> >r_a_tov + 1));
>  	if (time_left)
>  		BNX2FC_IO_DBG(io_req,
>  			      "Timed out in eh_abort waiting for
> abts_done");