* [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
@ 2019-09-06 20:26 Laurence Oberman
2019-09-06 20:31 ` Laurence Oberman
2019-09-08 1:34 ` cdupuis1
0 siblings, 2 replies; 11+ messages in thread
From: Laurence Oberman @ 2019-09-06 20:26 UTC (permalink / raw)
To: loberman, QLogic-Storage-Upstream, linux-scsi
The qla2xxx driver had this issue as well when the newer array
firmware returned the retry_delay_timer in the fcp_rsp.
The bnx2fc is not handling the masking of the scope bits either
so the retry_delay_timestamp value lands up being a large value
added to the timer timestamp delaying I/O for up to 27 Minutes.
This patch adds similar code to handle this to the
bnx2fc driver to avoid the huge delay.
Signed-off-by: Laurence Oberman <loberman@redhat.com>
---
drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 9e50e5b..39f4aeb 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd *io_req,
struct bnx2fc_rport *tgt = io_req->tgt;
struct scsi_cmnd *sc_cmd;
struct Scsi_Host *host;
+ u16 scope, qualifier = 0;
/* scsi_cmd_cmpl is called with tgt lock held */
@@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd *io_req,
if (io_req->cdb_status == SAM_STAT_TASK_SET_FULL ||
io_req->cdb_status == SAM_STAT_BUSY) {
+ /* Newer array firmware with BUSY or
+ * TASK_SET_FULL may return a status that needs
+ * the scope bits masked.
+ * Or a huge delay timestamp up to 27 minutes
+ * can result.
+ */
+ if (fcp_rsp->retry_delay_timer) {
+ /* Upper 2 bits */
+ scope = fcp_rsp->retry_delay_timer
+ & 0xC000;
+ /* Lower 14 bits */
+ qualifier = fcp_rsp->retry_delay_timer
+ & 0x3FFF;
+ }
+ if (scope > 0 && qualifier > 0 &&
+ qualifier <= 0x3FEF) {
/* Set the jiffies + retry_delay_timer * 100ms
for the rport/tgt */
- tgt->retry_delay_timestamp = jiffies +
- fcp_rsp->retry_delay_timer * HZ / 10;
+ tgt->retry_delay_timestamp = jiffies +
+ (qualifier * HZ / 10);
+ }
}
-
}
if (io_req->fcp_resid)
scsi_set_resid(sc_cmd, io_req->fcp_resid);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
2019-09-06 20:26 [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL Laurence Oberman
@ 2019-09-06 20:31 ` Laurence Oberman
2019-09-08 1:34 ` cdupuis1
1 sibling, 0 replies; 11+ messages in thread
From: Laurence Oberman @ 2019-09-06 20:31 UTC (permalink / raw)
To: QLogic-Storage-Upstream, linux-scsi
On Fri, 2019-09-06 at 16:26 -0400, Laurence Oberman wrote:
> The qla2xxx driver had this issue as well when the newer array
> firmware returned the retry_delay_timer in the fcp_rsp.
> The bnx2fc is not handling the masking of the scope bits either
> so the retry_delay_timestamp value lands up being a large value
> added to the timer timestamp delaying I/O for up to 27 Minutes.
> This patch adds similar code to handle this to the
> bnx2fc driver to avoid the huge delay.
>
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> ---
> drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
> b/drivers/scsi/bnx2fc/bnx2fc_io.c
> index 9e50e5b..39f4aeb 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> @@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct
> bnx2fc_cmd *io_req,
> struct bnx2fc_rport *tgt = io_req->tgt;
> struct scsi_cmnd *sc_cmd;
> struct Scsi_Host *host;
> + u16 scope, qualifier = 0;
>
>
> /* scsi_cmd_cmpl is called with tgt lock held */
> @@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct
> bnx2fc_cmd *io_req,
>
> if (io_req->cdb_status ==
> SAM_STAT_TASK_SET_FULL ||
> io_req->cdb_status == SAM_STAT_BUSY) {
> + /* Newer array firmware with BUSY or
> + * TASK_SET_FULL may return a status
> that needs
> + * the scope bits masked.
> + * Or a huge delay timestamp up to 27
> minutes
> + * can result.
> + */
> + if (fcp_rsp->retry_delay_timer) {
> + /* Upper 2 bits */
> + scope = fcp_rsp-
> >retry_delay_timer
> + & 0xC000;
> + /* Lower 14 bits */
> + qualifier = fcp_rsp-
> >retry_delay_timer
> + & 0x3FFF;
> + }
> + if (scope > 0 && qualifier > 0 &&
> + qualifier <= 0x3FEF) {
> /* Set the jiffies + retry_delay_timer
> * 100ms
> for the rport/tgt */
> - tgt->retry_delay_timestamp = jiffies +
> - fcp_rsp->retry_delay_timer * HZ
> / 10;
> + tgt->retry_delay_timestamp =
> jiffies +
> + (qualifier * HZ / 10);
> + }
> }
> -
> }
> if (io_req->fcp_resid)
> scsi_set_resid(sc_cmd, io_req->fcp_resid);
Hello
Please add
Reported-by: David Jeffery <djeffery@redhat.com>
Apologies forgot to add that.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
2019-09-06 20:26 [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL Laurence Oberman
2019-09-06 20:31 ` Laurence Oberman
@ 2019-09-08 1:34 ` cdupuis1
2019-09-09 11:52 ` [PATCH] V2. " Laurence Oberman
1 sibling, 1 reply; 11+ messages in thread
From: cdupuis1 @ 2019-09-08 1:34 UTC (permalink / raw)
To: Laurence Oberman, QLogic-Storage-Upstream, linux-scsi
On Fri, 2019-09-06 at 16:26 -0400, Laurence Oberman wrote:
> The qla2xxx driver had this issue as well when the newer array
> firmware returned the retry_delay_timer in the fcp_rsp.
> The bnx2fc is not handling the masking of the scope bits either
> so the retry_delay_timestamp value lands up being a large value
> added to the timer timestamp delaying I/O for up to 27 Minutes.
> This patch adds similar code to handle this to the
> bnx2fc driver to avoid the huge delay.
>
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> ---
> drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
> b/drivers/scsi/bnx2fc/bnx2fc_io.c
> index 9e50e5b..39f4aeb 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> @@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct
> bnx2fc_cmd *io_req,
> struct bnx2fc_rport *tgt = io_req->tgt;
> struct scsi_cmnd *sc_cmd;
> struct Scsi_Host *host;
> + u16 scope, qualifier = 0;
>
>
> /* scsi_cmd_cmpl is called with tgt lock held */
> @@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct
> bnx2fc_cmd *io_req,
>
> if (io_req->cdb_status ==
> SAM_STAT_TASK_SET_FULL ||
> io_req->cdb_status == SAM_STAT_BUSY) {
> + /* Newer array firmware with BUSY or
> + * TASK_SET_FULL may return a status
> that needs
> + * the scope bits masked.
> + * Or a huge delay timestamp up to 27
> minutes
> + * can result.
> + */
> + if (fcp_rsp->retry_delay_timer) {
> + /* Upper 2 bits */
> + scope = fcp_rsp-
> >retry_delay_timer
> + & 0xC000;
> + /* Lower 14 bits */
> + qualifier = fcp_rsp-
> >retry_delay_timer
> + & 0x3FFF;
> + }
> + if (scope > 0 && qualifier > 0 &&
> + qualifier <= 0x3FEF) {
> /* Set the jiffies + retry_delay_timer
> * 100ms
> for the rport/tgt */
> - tgt->retry_delay_timestamp = jiffies +
> - fcp_rsp->retry_delay_timer * HZ
> / 10;
> + tgt->retry_delay_timestamp =
> jiffies +
> + (qualifier * HZ / 10);
> + }
> }
> -
> }
> if (io_req->fcp_resid)
> scsi_set_resid(sc_cmd, io_req->fcp_resid);
What better thing to be doing than reviewing patches on a Saturday
evening. Looks good though I might suggest moving the indent of the
comment in the new if statement.
Reviewed-by: Chad Dupuis <cdupuis1@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] V2. bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
2019-09-08 1:34 ` cdupuis1
@ 2019-09-09 11:52 ` Laurence Oberman
2019-09-09 12:04 ` Laurence Oberman
0 siblings, 1 reply; 11+ messages in thread
From: Laurence Oberman @ 2019-09-09 11:52 UTC (permalink / raw)
To: cdupuis1, QLogic-Storage-Upstream, linux-scsi
On Sat, 2019-09-07 at 21:34 -0400, cdupuis1@gmail.com wrote:
> On Fri, 2019-09-06 at 16:26 -0400, Laurence Oberman wrote:
> > The qla2xxx driver had this issue as well when the newer array
> > firmware returned the retry_delay_timer in the fcp_rsp.
> > The bnx2fc is not handling the masking of the scope bits either
> > so the retry_delay_timestamp value lands up being a large value
> > added to the timer timestamp delaying I/O for up to 27 Minutes.
> > This patch adds similar code to handle this to the
> > bnx2fc driver to avoid the huge delay.
> >
> > Signed-off-by: Laurence Oberman <loberman@redhat.com>
> > ---
> > drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
> > 1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
> > b/drivers/scsi/bnx2fc/bnx2fc_io.c
> > index 9e50e5b..39f4aeb 100644
> > --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> > +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> > @@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct
> > bnx2fc_cmd *io_req,
> > struct bnx2fc_rport *tgt = io_req->tgt;
> > struct scsi_cmnd *sc_cmd;
> > struct Scsi_Host *host;
> > + u16 scope, qualifier = 0;
> >
> >
> > /* scsi_cmd_cmpl is called with tgt lock held */
> > @@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct
> > bnx2fc_cmd *io_req,
> >
> > if (io_req->cdb_status ==
> > SAM_STAT_TASK_SET_FULL ||
> > io_req->cdb_status == SAM_STAT_BUSY) {
> > + /* Newer array firmware with BUSY or
> > + * TASK_SET_FULL may return a status
> > that needs
> > + * the scope bits masked.
> > + * Or a huge delay timestamp up to 27
> > minutes
> > + * can result.
> > + */
> > + if (fcp_rsp->retry_delay_timer) {
> > + /* Upper 2 bits */
> > + scope = fcp_rsp-
> > > retry_delay_timer
> >
> > + & 0xC000;
> > + /* Lower 14 bits */
> > + qualifier = fcp_rsp-
> > > retry_delay_timer
> >
> > + & 0x3FFF;
> > + }
> > + if (scope > 0 && qualifier > 0 &&
> > + qualifier <= 0x3FEF) {
> > /* Set the jiffies + retry_delay_timer
> > * 100ms
> > for the rport/tgt */
> > - tgt->retry_delay_timestamp = jiffies +
> > - fcp_rsp->retry_delay_timer * HZ
> > / 10;
> > + tgt->retry_delay_timestamp =
> > jiffies +
> > + (qualifier * HZ / 10);
> > + }
> > }
> > -
> > }
> > if (io_req->fcp_resid)
> > scsi_set_resid(sc_cmd, io_req->fcp_resid);
>
> What better thing to be doing than reviewing patches on a Saturday
> evening. Looks good though I might suggest moving the indent of the
> comment in the new if statement.
>
> Reviewed-by: Chad Dupuis <cdupuis1@gmail.com>
>
The qla2xxx driver had this issue as well when the newer array
firmware returned the retry_delay_timer in the fcp_rsp.
The bnx2fc is not handling the masking of the scope bits either
so the retry_delay_timestamp value lands up being a large value
added to the timer timestamp delaying I/O for up to 27 Minutes.
This patch adds similar code to handle this to the
bnx2fc driver to avoid the huge delay.
V2. Indent comments as suggested
Signed-off-by: Laurence Oberman <loberman@redhat.com>
Reported-by: David Jeffery <djeffery@redhat.com>
---
drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 9e50e5b..39f4aeb 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct
bnx2fc_cmd *io_req,
struct bnx2fc_rport *tgt = io_req->tgt;
struct scsi_cmnd *sc_cmd;
struct Scsi_Host *host;
+ u16 scope, qualifier = 0;
/* scsi_cmd_cmpl is called with tgt lock held */
@@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct
bnx2fc_cmd *io_req,
if (io_req->cdb_status ==
SAM_STAT_TASK_SET_FULL ||
io_req->cdb_status == SAM_STAT_BUSY) {
+ /* Newer array firmware with BUSY or
+ * TASK_SET_FULL may return a status
that needs
+ * the scope bits masked.
+ * Or a huge delay timestamp up to 27
minutes
+ * can result.
+ */
+ if (fcp_rsp->retry_delay_timer) {
+ /* Upper 2 bits */
+ scope = fcp_rsp-
>retry_delay_timer
+ & 0xC000;
+ /* Lower 14 bits */
+ qualifier = fcp_rsp-
>retry_delay_timer
+ & 0x3FFF;
+ }
+ if (scope > 0 && qualifier > 0 &&
+ qualifier <= 0x3FEF) {
/* Set the jiffies +
retry_delay_timer * 100ms
for the rport/tgt */
- tgt->retry_delay_timestamp = jiffies +
- fcp_rsp->retry_delay_timer * HZ
/ 10;
+ tgt->retry_delay_timestamp =
jiffies +
+ (qualifier * HZ / 10);
+ }
}
-
}
if (io_req->fcp_resid)
scsi_set_resid(sc_cmd, io_req->fcp_resid);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] V2. bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
2019-09-09 11:52 ` [PATCH] V2. " Laurence Oberman
@ 2019-09-09 12:04 ` Laurence Oberman
0 siblings, 0 replies; 11+ messages in thread
From: Laurence Oberman @ 2019-09-09 12:04 UTC (permalink / raw)
To: cdupuis1, QLogic-Storage-Upstream, linux-scsi
On Mon, 2019-09-09 at 07:52 -0400, Laurence Oberman wrote:
> On Sat, 2019-09-07 at 21:34 -0400, cdupuis1@gmail.com wrote:
> > On Fri, 2019-09-06 at 16:26 -0400, Laurence Oberman wrote:
> > > The qla2xxx driver had this issue as well when the newer array
> > > firmware returned the retry_delay_timer in the fcp_rsp.
> > > The bnx2fc is not handling the masking of the scope bits either
> > > so the retry_delay_timestamp value lands up being a large value
> > > added to the timer timestamp delaying I/O for up to 27 Minutes.
> > > This patch adds similar code to handle this to the
> > > bnx2fc driver to avoid the huge delay.
> > >
> > > Signed-off-by: Laurence Oberman <loberman@redhat.com>
> > > ---
> > > drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
> > > 1 file changed, 20 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
> > > b/drivers/scsi/bnx2fc/bnx2fc_io.c
> > > index 9e50e5b..39f4aeb 100644
> > > --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> > > +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> > > @@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct
> > > bnx2fc_cmd *io_req,
> > > struct bnx2fc_rport *tgt = io_req->tgt;
> > > struct scsi_cmnd *sc_cmd;
> > > struct Scsi_Host *host;
> > > + u16 scope, qualifier = 0;
> > >
> > >
> > > /* scsi_cmd_cmpl is called with tgt lock held */
> > > @@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct
> > > bnx2fc_cmd *io_req,
> > >
> > > if (io_req->cdb_status ==
> > > SAM_STAT_TASK_SET_FULL ||
> > > io_req->cdb_status == SAM_STAT_BUSY) {
> > > + /* Newer array firmware with BUSY or
> > > + * TASK_SET_FULL may return a status
> > > that needs
> > > + * the scope bits masked.
> > > + * Or a huge delay timestamp up to 27
> > > minutes
> > > + * can result.
> > > + */
> > > + if (fcp_rsp->retry_delay_timer) {
> > > + /* Upper 2 bits */
> > > + scope = fcp_rsp-
> > > > retry_delay_timer
> > >
> > > + & 0xC000;
> > > + /* Lower 14 bits */
> > > + qualifier = fcp_rsp-
> > > > retry_delay_timer
> > >
> > > + & 0x3FFF;
> > > + }
> > > + if (scope > 0 && qualifier > 0 &&
> > > + qualifier <= 0x3FEF) {
> > > /* Set the jiffies + retry_delay_timer
> > > * 100ms
> > > for the rport/tgt */
> > > - tgt->retry_delay_timestamp = jiffies +
> > > - fcp_rsp->retry_delay_timer * HZ
> > > / 10;
> > > + tgt->retry_delay_timestamp =
> > > jiffies +
> > > + (qualifier * HZ / 10);
> > > + }
> > > }
> > > -
> > > }
> > > if (io_req->fcp_resid)
> > > scsi_set_resid(sc_cmd, io_req->fcp_resid);
> >
> > What better thing to be doing than reviewing patches on a Saturday
> > evening. Looks good though I might suggest moving the indent of
> > the
> > comment in the new if statement.
> >
> > Reviewed-by: Chad Dupuis <cdupuis1@gmail.com>
> >
>
> The qla2xxx driver had this issue as well when the newer array
> firmware returned the retry_delay_timer in the fcp_rsp.
> The bnx2fc is not handling the masking of the scope bits either
> so the retry_delay_timestamp value lands up being a large value
> added to the timer timestamp delaying I/O for up to 27 Minutes.
> This patch adds similar code to handle this to the
> bnx2fc driver to avoid the huge delay.
>
> V2. Indent comments as suggested
>
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> Reported-by: David Jeffery <djeffery@redhat.com>
>
> ---
> drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
> b/drivers/scsi/bnx2fc/bnx2fc_io.c
> index 9e50e5b..39f4aeb 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> @@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct
> bnx2fc_cmd *io_req,
> struct bnx2fc_rport *tgt = io_req->tgt;
> struct scsi_cmnd *sc_cmd;
> struct Scsi_Host *host;
> + u16 scope, qualifier = 0;
>
>
> /* scsi_cmd_cmpl is called with tgt lock held */
> @@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct
> bnx2fc_cmd *io_req,
>
> if (io_req->cdb_status ==
> SAM_STAT_TASK_SET_FULL ||
> io_req->cdb_status == SAM_STAT_BUSY) {
> + /* Newer array firmware with BUSY or
> + * TASK_SET_FULL may return a status
> that needs
> + * the scope bits masked.
> + * Or a huge delay timestamp up to 27
> minutes
> + * can result.
> + */
> + if (fcp_rsp->retry_delay_timer) {
> + /* Upper 2 bits */
> + scope = fcp_rsp-
> > retry_delay_timer
>
> + & 0xC000;
> + /* Lower 14 bits */
> + qualifier = fcp_rsp-
> > retry_delay_timer
>
> + & 0x3FFF;
> + }
> + if (scope > 0 && qualifier > 0 &&
> + qualifier <= 0x3FEF) {
> /* Set the jiffies +
> retry_delay_timer * 100ms
> for the rport/tgt */
> - tgt->retry_delay_timestamp = jiffies +
> - fcp_rsp->retry_delay_timer * HZ
> / 10;
> + tgt->retry_delay_timestamp =
> jiffies +
> + (qualifier * HZ / 10);
> + }
> }
> -
> }
> if (io_req->fcp_resid)
> scsi_set_resid(sc_cmd, io_req->fcp_resid);
I will send a V2 from git this got munged by the mailer on the reply.
Also comments line landed up exceeding 79 column width
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
@ 2019-09-09 12:05 Laurence Oberman
2019-09-09 17:40 ` Lee Duncan
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Laurence Oberman @ 2019-09-09 12:05 UTC (permalink / raw)
To: loberman, QLogic-Storage-Upstream, linux-scsi
The qla2xxx driver had this issue as well when the newer array
firmware returned the retry_delay_timer in the fcp_rsp.
The bnx2fc is not handling the masking of the scope bits either
so the retry_delay_timestamp value lands up being a large value
added to the timer timestamp delaying I/O for up to 27 Minutes.
This patch adds similar code to handle this to the
bnx2fc driver to avoid the huge delay.
V2. Indent comments as suggested
Signed-off-by: Laurence Oberman <loberman@redhat.com>
---
drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 9e50e5b..39f4aeb 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd *io_req,
struct bnx2fc_rport *tgt = io_req->tgt;
struct scsi_cmnd *sc_cmd;
struct Scsi_Host *host;
+ u16 scope, qualifier = 0;
/* scsi_cmd_cmpl is called with tgt lock held */
@@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd *io_req,
if (io_req->cdb_status == SAM_STAT_TASK_SET_FULL ||
io_req->cdb_status == SAM_STAT_BUSY) {
+ /* Newer array firmware with BUSY or
+ * TASK_SET_FULL may return a status that needs
+ * the scope bits masked.
+ * Or a huge delay timestamp up to 27 minutes
+ * can result.
+ */
+ if (fcp_rsp->retry_delay_timer) {
+ /* Upper 2 bits */
+ scope = fcp_rsp->retry_delay_timer
+ & 0xC000;
+ /* Lower 14 bits */
+ qualifier = fcp_rsp->retry_delay_timer
+ & 0x3FFF;
+ }
+ if (scope > 0 && qualifier > 0 &&
+ qualifier <= 0x3FEF) {
/* Set the jiffies +
retry_delay_timer * 100ms
for the rport/tgt
*/
- tgt->retry_delay_timestamp = jiffies +
- fcp_rsp->retry_delay_timer * HZ / 10;
+ tgt->retry_delay_timestamp = jiffies +
+ (qualifier * HZ / 10);
+ }
}
-
}
if (io_req->fcp_resid)
scsi_set_resid(sc_cmd, io_req->fcp_resid);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
2019-09-09 12:05 [PATCH] " Laurence Oberman
@ 2019-09-09 17:40 ` Lee Duncan
2019-09-11 0:19 ` cdupuis1
2019-09-11 2:21 ` Martin K. Petersen
2 siblings, 0 replies; 11+ messages in thread
From: Lee Duncan @ 2019-09-09 17:40 UTC (permalink / raw)
To: Laurence Oberman, QLogic-Storage-Upstream, linux-scsi
On 9/9/19 5:05 AM, Laurence Oberman wrote:
> The qla2xxx driver had this issue as well when the newer array
> firmware returned the retry_delay_timer in the fcp_rsp.
> The bnx2fc is not handling the masking of the scope bits either
> so the retry_delay_timestamp value lands up being a large value
> added to the timer timestamp delaying I/O for up to 27 Minutes.
> This patch adds similar code to handle this to the
> bnx2fc driver to avoid the huge delay.
>
> V2. Indent comments as suggested
>
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> ---
> drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
> index 9e50e5b..39f4aeb 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> @@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd *io_req,
> struct bnx2fc_rport *tgt = io_req->tgt;
> struct scsi_cmnd *sc_cmd;
> struct Scsi_Host *host;
> + u16 scope, qualifier = 0;
>
>
> /* scsi_cmd_cmpl is called with tgt lock held */
> @@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd *io_req,
>
> if (io_req->cdb_status == SAM_STAT_TASK_SET_FULL ||
> io_req->cdb_status == SAM_STAT_BUSY) {
> + /* Newer array firmware with BUSY or
> + * TASK_SET_FULL may return a status that needs
> + * the scope bits masked.
> + * Or a huge delay timestamp up to 27 minutes
> + * can result.
> + */
> + if (fcp_rsp->retry_delay_timer) {
> + /* Upper 2 bits */
> + scope = fcp_rsp->retry_delay_timer
> + & 0xC000;
> + /* Lower 14 bits */
> + qualifier = fcp_rsp->retry_delay_timer
> + & 0x3FFF;
> + }
> + if (scope > 0 && qualifier > 0 &&
> + qualifier <= 0x3FEF) {
> /* Set the jiffies +
> retry_delay_timer * 100ms
> for the rport/tgt
> */
> - tgt->retry_delay_timestamp = jiffies +
> - fcp_rsp->retry_delay_timer * HZ / 10;
> + tgt->retry_delay_timestamp = jiffies +
> + (qualifier * HZ / 10);
> + }
> }
> -
> }
> if (io_req->fcp_resid)
> scsi_set_resid(sc_cmd, io_req->fcp_resid);
>
Reviewed-by: Lee Duncan <lduncan@suse.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
2019-09-09 12:05 [PATCH] " Laurence Oberman
2019-09-09 17:40 ` Lee Duncan
@ 2019-09-11 0:19 ` cdupuis1
2019-09-11 2:21 ` Martin K. Petersen
2 siblings, 0 replies; 11+ messages in thread
From: cdupuis1 @ 2019-09-11 0:19 UTC (permalink / raw)
To: Laurence Oberman, QLogic-Storage-Upstream, linux-scsi
On Mon, 2019-09-09 at 08:05 -0400, Laurence Oberman wrote:
> The qla2xxx driver had this issue as well when the newer array
> firmware returned the retry_delay_timer in the fcp_rsp.
> The bnx2fc is not handling the masking of the scope bits either
> so the retry_delay_timestamp value lands up being a large value
> added to the timer timestamp delaying I/O for up to 27 Minutes.
> This patch adds similar code to handle this to the
> bnx2fc driver to avoid the huge delay.
>
> V2. Indent comments as suggested
>
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> ---
> drivers/scsi/bnx2fc/bnx2fc_io.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c
> b/drivers/scsi/bnx2fc/bnx2fc_io.c
> index 9e50e5b..39f4aeb 100644
> --- a/drivers/scsi/bnx2fc/bnx2fc_io.c
> +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
> @@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct
> bnx2fc_cmd *io_req,
> struct bnx2fc_rport *tgt = io_req->tgt;
> struct scsi_cmnd *sc_cmd;
> struct Scsi_Host *host;
> + u16 scope, qualifier = 0;
>
>
> /* scsi_cmd_cmpl is called with tgt lock held */
> @@ -1997,12 +1998,28 @@ void bnx2fc_process_scsi_cmd_compl(struct
> bnx2fc_cmd *io_req,
>
> if (io_req->cdb_status ==
> SAM_STAT_TASK_SET_FULL ||
> io_req->cdb_status == SAM_STAT_BUSY) {
> + /* Newer array firmware with BUSY or
> + * TASK_SET_FULL may return a status
> that needs
> + * the scope bits masked.
> + * Or a huge delay timestamp up to 27
> minutes
> + * can result.
> + */
> + if (fcp_rsp->retry_delay_timer) {
> + /* Upper 2 bits */
> + scope = fcp_rsp-
> >retry_delay_timer
> + & 0xC000;
> + /* Lower 14 bits */
> + qualifier = fcp_rsp-
> >retry_delay_timer
> + & 0x3FFF;
> + }
> + if (scope > 0 && qualifier > 0 &&
> + qualifier <= 0x3FEF) {
> /* Set the jiffies +
> retry_delay_timer * 100ms
> for the rport/tgt
> */
> - tgt->retry_delay_timestamp = jiffies +
> - fcp_rsp->retry_delay_timer * HZ
> / 10;
> + tgt->retry_delay_timestamp =
> jiffies +
> + (qualifier * HZ / 10);
> + }
> }
> -
> }
> if (io_req->fcp_resid)
> scsi_set_resid(sc_cmd, io_req->fcp_resid);
V2 looks good.
Reviewed-by: Chad Dupuis <cdupuis1@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
2019-09-09 12:05 [PATCH] " Laurence Oberman
2019-09-09 17:40 ` Lee Duncan
2019-09-11 0:19 ` cdupuis1
@ 2019-09-11 2:21 ` Martin K. Petersen
2 siblings, 0 replies; 11+ messages in thread
From: Martin K. Petersen @ 2019-09-11 2:21 UTC (permalink / raw)
To: Laurence Oberman; +Cc: QLogic-Storage-Upstream, linux-scsi
Laurence,
> The qla2xxx driver had this issue as well when the newer array
> firmware returned the retry_delay_timer in the fcp_rsp. The bnx2fc is
> not handling the masking of the scope bits either so the
> retry_delay_timestamp value lands up being a large value added to the
> timer timestamp delaying I/O for up to 27 Minutes. This patch adds
> similar code to handle this to the bnx2fc driver to avoid the huge
> delay.
Does not apply to 5.4/scsi-queue.
When you resubmit, please use git send-email -v3 [...] and put the
changelog commentary after a "---" separator.
> V2. Indent comments as suggested
>
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
@ 2019-09-11 13:56 Laurence Oberman
2019-09-24 2:19 ` Martin K. Petersen
0 siblings, 1 reply; 11+ messages in thread
From: Laurence Oberman @ 2019-09-11 13:56 UTC (permalink / raw)
To: loberman, djeffery, cdupuis1, lduncan, martin.petersen,
QLogic-Storage-Upstream, linux-scsi
The qla2xxx driver had this issue as well when the newer array
firmware returned the retry_delay_timer in the fcp_rsp.
The bnx2fc is not handling the masking of the scope bits either
so the retry_delay_timestamp value lands up being a large value
added to the timer timestamp delaying I/O for up to 27 Minutes.
This patch adds similar code to handle this to the
bnx2fc driver to avoid the huge delay.
---
V2. Fix Indent for comments (Chad)
V3. Kbuild robot reported uninitialized variable scope
Initialize scope to 0
Signed-off-by: Laurence Oberman <loberman@redhat.com>
Reported-by: David Jeffery <djeffery@redhat.com>
Reported-by: kbuild test robot <lkp@intel.com>
---
drivers/scsi/bnx2fc/bnx2fc_io.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 9e50e5b..579b88d 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -1928,6 +1928,7 @@ void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd *io_req,
struct bnx2fc_rport *tgt = io_req->tgt;
struct scsi_cmnd *sc_cmd;
struct Scsi_Host *host;
+ u16 scope = 0, qualifier = 0;
/* scsi_cmd_cmpl is called with tgt lock held */
@@ -1997,12 +1998,30 @@ void bnx2fc_process_scsi_cmd_compl(struct bnx2fc_cmd *io_req,
if (io_req->cdb_status == SAM_STAT_TASK_SET_FULL ||
io_req->cdb_status == SAM_STAT_BUSY) {
- /* Set the jiffies + retry_delay_timer * 100ms
- for the rport/tgt */
- tgt->retry_delay_timestamp = jiffies +
- fcp_rsp->retry_delay_timer * HZ / 10;
+ /* Newer array firmware with BUSY or
+ * TASK_SET_FULL may return a status that needs
+ * the scope bits masked.
+ * Or a huge delay timestamp up to 27 minutes
+ * can result.
+ */
+ if (fcp_rsp->retry_delay_timer) {
+ /* Upper 2 bits */
+ scope = fcp_rsp->retry_delay_timer
+ & 0xC000;
+ /* Lower 14 bits */
+ qualifier = fcp_rsp->retry_delay_timer
+ & 0x3FFF;
+ }
+ if (scope > 0 && qualifier > 0 &&
+ qualifier <= 0x3FEF) {
+ /* Set the jiffies +
+ * retry_delay_timer * 100ms
+ * for the rport/tgt
+ */
+ tgt->retry_delay_timestamp = jiffies +
+ (qualifier * HZ / 10);
+ }
}
-
}
if (io_req->fcp_resid)
scsi_set_resid(sc_cmd, io_req->fcp_resid);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL
2019-09-11 13:56 Laurence Oberman
@ 2019-09-24 2:19 ` Martin K. Petersen
0 siblings, 0 replies; 11+ messages in thread
From: Martin K. Petersen @ 2019-09-24 2:19 UTC (permalink / raw)
To: Laurence Oberman
Cc: djeffery, cdupuis1, lduncan, martin.petersen,
QLogic-Storage-Upstream, linux-scsi
Laurence,
> The qla2xxx driver had this issue as well when the newer array
> firmware returned the retry_delay_timer in the fcp_rsp.
> The bnx2fc is not handling the masking of the scope bits either
> so the retry_delay_timestamp value lands up being a large value
> added to the timer timestamp delaying I/O for up to 27 Minutes.
> This patch adds similar code to handle this to the
> bnx2fc driver to avoid the huge delay.
> ---
> V2. Fix Indent for comments (Chad)
> V3. Kbuild robot reported uninitialized variable scope
> Initialize scope to 0
>
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> Reported-by: David Jeffery <djeffery@redhat.com>
> Reported-by: kbuild test robot <lkp@intel.com>
> ---
> drivers/scsi/bnx2fc/bnx2fc_io.c | 29 ++++++++++++++++++++++++-----
> 1 file changed, 24 insertions(+), 5 deletions(-)
Applied to 5.4/scsi-fixes.
Please read Documentation/process/submitting-patches.rst. Your patch got
all mangled since you put tags after the --- separator.
Also, please use the -vN option when submitting updated patches.
Thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-09-24 2:20 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06 20:26 [PATCH] bnx2fc: Handle scope bits when array returns BUSY or TASK_SET_FULL Laurence Oberman
2019-09-06 20:31 ` Laurence Oberman
2019-09-08 1:34 ` cdupuis1
2019-09-09 11:52 ` [PATCH] V2. " Laurence Oberman
2019-09-09 12:04 ` Laurence Oberman
2019-09-09 12:05 [PATCH] " Laurence Oberman
2019-09-09 17:40 ` Lee Duncan
2019-09-11 0:19 ` cdupuis1
2019-09-11 2:21 ` Martin K. Petersen
2019-09-11 13:56 Laurence Oberman
2019-09-24 2:19 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.