* [PATCH v3] qla2xxx: Return EBUSY on fcport deletion
@ 2020-10-12 17:35 Daniel Wagner
2020-10-12 18:14 ` [EXT] " Arun Easi
2020-10-12 23:59 ` Finn Thain
0 siblings, 2 replies; 6+ messages in thread
From: Daniel Wagner @ 2020-10-12 17:35 UTC (permalink / raw)
To: Nilesh Javali, Arun Easi; +Cc: linux-scsi, linux-kernel, Daniel Wagner
When the fcport is about to be deleted we should return EBUSY instead
of ENODEV. Only for EBUSY the request will be requeued in a multipath
setup.
Also in case we have a valid qpair but the firmware has not yet
started return EBUSY to avoid dropping the request.
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
v3: simplify test logic as suggested by Arun.
v2: rebased on mkp/staging
drivers/scsi/qla2xxx/qla_nvme.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index 2cd9bd288910..1fa457a5736e 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -555,10 +555,12 @@ static int qla_nvme_post_cmd(struct nvme_fc_local_port *lport,
fcport = qla_rport->fcport;
- if (!qpair || !fcport || (qpair && !qpair->fw_started) ||
- (fcport && fcport->deleted))
+ if (!qpair || !fcport)
return -ENODEV;
+ if (!qpair->fw_started || fcport->deleted)
+ return -EBUSY;
+
vha = fcport->vha;
if (!(fcport->nvme_flag & NVME_FLAG_REGISTERED))
--
2.16.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [EXT] [PATCH v3] qla2xxx: Return EBUSY on fcport deletion
2020-10-12 17:35 [PATCH v3] qla2xxx: Return EBUSY on fcport deletion Daniel Wagner
@ 2020-10-12 18:14 ` Arun Easi
2020-10-12 23:59 ` Finn Thain
1 sibling, 0 replies; 6+ messages in thread
From: Arun Easi @ 2020-10-12 18:14 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Nilesh Javali, linux-scsi, linux-kernel
On Mon, 12 Oct 2020, 10:35am, Daniel Wagner wrote:
> ----------------------------------------------------------------------
> When the fcport is about to be deleted we should return EBUSY instead
> of ENODEV. Only for EBUSY the request will be requeued in a multipath
> setup.
>
> Also in case we have a valid qpair but the firmware has not yet
> started return EBUSY to avoid dropping the request.
>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>
> v3: simplify test logic as suggested by Arun.
> v2: rebased on mkp/staging
>
> drivers/scsi/qla2xxx/qla_nvme.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
> index 2cd9bd288910..1fa457a5736e 100644
> --- a/drivers/scsi/qla2xxx/qla_nvme.c
> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> @@ -555,10 +555,12 @@ static int qla_nvme_post_cmd(struct nvme_fc_local_port *lport,
>
> fcport = qla_rport->fcport;
>
> - if (!qpair || !fcport || (qpair && !qpair->fw_started) ||
> - (fcport && fcport->deleted))
> + if (!qpair || !fcport)
> return -ENODEV;
>
> + if (!qpair->fw_started || fcport->deleted)
> + return -EBUSY;
> +
> vha = fcport->vha;
>
> if (!(fcport->nvme_flag & NVME_FLAG_REGISTERED))
>
Thanks Daniel.
Reviewed-by: Arun Easi <aeasi@marvell.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] qla2xxx: Return EBUSY on fcport deletion
2020-10-12 17:35 [PATCH v3] qla2xxx: Return EBUSY on fcport deletion Daniel Wagner
2020-10-12 18:14 ` [EXT] " Arun Easi
@ 2020-10-12 23:59 ` Finn Thain
2020-10-13 6:52 ` Daniel Wagner
1 sibling, 1 reply; 6+ messages in thread
From: Finn Thain @ 2020-10-12 23:59 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Nilesh Javali, Arun Easi, linux-scsi, linux-kernel
On Mon, 12 Oct 2020, Daniel Wagner wrote:
> When the fcport is about to be deleted we should return EBUSY instead
> of ENODEV. Only for EBUSY the request will be requeued in a multipath
> setup.
>
> Also in case we have a valid qpair but the firmware has not yet
> started return EBUSY to avoid dropping the request.
>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>
> v3: simplify test logic as suggested by Arun.
Not exactly a "simplification": there was a change of behaviour between v2
and v3. It seems the commit log no longer reflects the code.
> v2: rebased on mkp/staging
>
> drivers/scsi/qla2xxx/qla_nvme.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
> index 2cd9bd288910..1fa457a5736e 100644
> --- a/drivers/scsi/qla2xxx/qla_nvme.c
> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> @@ -555,10 +555,12 @@ static int qla_nvme_post_cmd(struct nvme_fc_local_port *lport,
>
> fcport = qla_rport->fcport;
>
> - if (!qpair || !fcport || (qpair && !qpair->fw_started) ||
> - (fcport && fcport->deleted))
> + if (!qpair || !fcport)
> return -ENODEV;
>
> + if (!qpair->fw_started || fcport->deleted)
> + return -EBUSY;
> +
> vha = fcport->vha;
>
> if (!(fcport->nvme_flag & NVME_FLAG_REGISTERED))
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] qla2xxx: Return EBUSY on fcport deletion
2020-10-12 23:59 ` Finn Thain
@ 2020-10-13 6:52 ` Daniel Wagner
2020-10-14 0:57 ` Finn Thain
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Wagner @ 2020-10-13 6:52 UTC (permalink / raw)
To: Finn Thain; +Cc: Nilesh Javali, Arun Easi, linux-scsi, linux-kernel
On Tue, Oct 13, 2020 at 10:59:18AM +1100, Finn Thain wrote:
>
> On Mon, 12 Oct 2020, Daniel Wagner wrote:
>
> > When the fcport is about to be deleted we should return EBUSY instead
> > of ENODEV. Only for EBUSY the request will be requeued in a multipath
> > setup.
> >
> > Also in case we have a valid qpair but the firmware has not yet
> > started return EBUSY to avoid dropping the request.
> >
> > Signed-off-by: Daniel Wagner <dwagner@suse.de>
> > ---
> >
> > v3: simplify test logic as suggested by Arun.
>
> Not exactly a "simplification": there was a change of behaviour between v2
> and v3. It seems the commit log no longer reflects the code.
How so? I am struggling to see how it could be a change in behavior. But
then I sometimes fail at simple logic ;)
v2 and v3 will return ENODEV if qpair or fcport are invalid and for
EBUSY one of the other condition needs be true. The difference between
v2 and v3 should only be the order how tests are executed. The outcome
should be the same.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] qla2xxx: Return EBUSY on fcport deletion
2020-10-13 6:52 ` Daniel Wagner
@ 2020-10-14 0:57 ` Finn Thain
2020-10-14 7:06 ` Daniel Wagner
0 siblings, 1 reply; 6+ messages in thread
From: Finn Thain @ 2020-10-14 0:57 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Nilesh Javali, Arun Easi, linux-scsi, linux-kernel
On Tue, 13 Oct 2020, Daniel Wagner wrote:
> On Tue, Oct 13, 2020 at 10:59:18AM +1100, Finn Thain wrote:
> >
> > On Mon, 12 Oct 2020, Daniel Wagner wrote:
> >
> > > When the fcport is about to be deleted we should return EBUSY
> > > instead of ENODEV. Only for EBUSY the request will be requeued in a
> > > multipath setup.
> > >
> > > Also in case we have a valid qpair but the firmware has not yet
> > > started return EBUSY to avoid dropping the request.
> > >
> > > Signed-off-by: Daniel Wagner <dwagner@suse.de>
> > > ---
> > >
> > > v3: simplify test logic as suggested by Arun.
> >
> > Not exactly a "simplification": there was a change of behaviour
> > between v2 and v3. It seems the commit log no longer reflects the
> > code.
>
> How so? I am struggling to see how it could be a change in behavior. But
> then I sometimes fail at simple logic ;)
>
Me too, so I confirmed the result by executing the code snippets.
> v2 and v3 will return ENODEV if qpair or fcport are invalid and for
> EBUSY one of the other condition needs be true. The difference between
> v2 and v3 should only be the order how tests are executed. The outcome
> should be the same.
>
Here's a truth table for v2:
qpair fw_started fcport deleted result
---------------------------------------
F X F X -ENODEV
F F T F -ENODEV
F F T T -EBUSY *
F T T F -ENODEV
F T T T -EBUSY *
T F F X -EBUSY *
T F T X -EBUSY
T T F X -ENODEV
T T T F neither
T T T T -EBUSY
Here's a truth table for v3:
qpair fw_started fcport deleted result
---------------------------------------
F X F X -ENODEV
F F T F -ENODEV
F F T T -ENODEV *
F T T F -ENODEV
F T T T -ENODEV *
T F F X -ENODEV *
T F T X -EBUSY
T T F X -ENODEV
T T T F neither
T T T T -EBUSY
The asterisks mark the changed rows.
I don't know whether the changes in v3 are desirable or not, I was just
pointing out that the commit log ("valid qpair but the firmware has not
yet started return EBUSY") now seems to disagree with the code.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] qla2xxx: Return EBUSY on fcport deletion
2020-10-14 0:57 ` Finn Thain
@ 2020-10-14 7:06 ` Daniel Wagner
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Wagner @ 2020-10-14 7:06 UTC (permalink / raw)
To: Finn Thain; +Cc: Nilesh Javali, Arun Easi, linux-scsi, linux-kernel
Hi Finn,
On Wed, Oct 14, 2020 at 11:57:04AM +1100, Finn Thain wrote:
>
> On Tue, 13 Oct 2020, Daniel Wagner wrote:
> > How so? I am struggling to see how it could be a change in behavior. But
> > then I sometimes fail at simple logic ;)
> >
>
> Me too, so I confirmed the result by executing the code snippets.
Thanks for taking the time to explain it to me!
> I don't know whether the changes in v3 are desirable or not, I was just
> pointing out that the commit log ("valid qpair but the firmware has not
> yet started return EBUSY") now seems to disagree with the code.
I see where I did my thinko. In v3 we have more ENODEV due to the fact
that we test the pointers first (qpair, fpcort). If either of them is
invalid we get the ENODEV. Actually, it's makes more sense and is likely
to be 'more correct'.
Anyway, let me update the commit log.
Thanks a lot!
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-14 7:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-12 17:35 [PATCH v3] qla2xxx: Return EBUSY on fcport deletion Daniel Wagner
2020-10-12 18:14 ` [EXT] " Arun Easi
2020-10-12 23:59 ` Finn Thain
2020-10-13 6:52 ` Daniel Wagner
2020-10-14 0:57 ` Finn Thain
2020-10-14 7:06 ` Daniel Wagner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).