[PATCH v1] qla2xxx: Allow nvme report port registration

[PATCH v1] qla2xxx: Allow nvme report port registration

[Daniel Wagner]

Move the both ONLINE state check into qla2x00_update_fcport and call
both register port register functions.

Currently, qla2x00_reg_remote_port and qla_nvme_register_remote check
the state if it is ONLINE. If it not, the state is set to ONLINE and
the function is executed.

qla2x00_reg_remote_port is called before qla_nvme_register_remote and
hence qla_nvme_register_remote will always bail out and never register
a nvme remote port.

Fixes: 6a45c8e137d4 ("scsi: qla2xxx: Fix disk failure to rediscover")
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Cc: Quinn Tran <qutran@marvell.com>
Cc: Nilesh Javali <njavali@marvell.com>
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 drivers/scsi/qla2xxx/qla_init.c | 33 ++++++++++++++++-----------------
 drivers/scsi/qla2xxx/qla_nvme.c |  5 -----
 2 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 3f3417a3e891..71b804cfd808 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -5764,11 +5764,6 @@ qla2x00_reg_remote_port(scsi_qla_host_t *vha, fc_port_t *fcport)
 	struct fc_rport *rport;
 	unsigned long flags;
 
-	if (atomic_read(&fcport->state) == FCS_ONLINE)
-		return;
-
-	qla2x00_set_fcport_state(fcport, FCS_ONLINE);
-
 	rport_ids.node_name = wwn_to_u64(fcport->node_name);
 	rport_ids.port_name = wwn_to_u64(fcport->port_name);
 	rport_ids.port_id = fcport->d_id.b.domain << 16 |
@@ -5864,18 +5859,25 @@ qla2x00_update_fcport(scsi_qla_host_t *vha, fc_port_t *fcport)
 
 	qla24xx_update_fcport_fcp_prio(vha, fcport);
 
+	if (atomic_read(&fcport->state) != FCS_ONLINE) {
+		qla2x00_set_fcport_state(fcport, FCS_ONLINE);
+
+		switch (vha->host->active_mode) {
+		case MODE_INITIATOR:
+		case MODE_DUAL:
+			qla2x00_reg_remote_port(vha, fcport);
+			break;
+		default:
+			break;
+		}
+
+		if (NVME_TARGET(vha->hw, fcport))
+			qla_nvme_register_remote(vha, fcport);
+	}
+
 	switch (vha->host->active_mode) {
-	case MODE_INITIATOR:
-		qla2x00_reg_remote_port(vha, fcport);
-		break;
 	case MODE_TARGET:
-		qla2x00_set_fcport_state(fcport, FCS_ONLINE);
-		if (!vha->vha_tgt.qla_tgt->tgt_stop &&
-			!vha->vha_tgt.qla_tgt->tgt_stopped)
-			qlt_fc_port_added(vha, fcport);
-		break;
 	case MODE_DUAL:
-		qla2x00_reg_remote_port(vha, fcport);
 		if (!vha->vha_tgt.qla_tgt->tgt_stop &&
 			!vha->vha_tgt.qla_tgt->tgt_stopped)
 			qlt_fc_port_added(vha, fcport);
@@ -5884,9 +5886,6 @@ qla2x00_update_fcport(scsi_qla_host_t *vha, fc_port_t *fcport)
 		break;
 	}
 
-	if (NVME_TARGET(vha->hw, fcport))
-		qla_nvme_register_remote(vha, fcport);
-
 	if (IS_IIDMA_CAPABLE(vha->hw) && vha->hw->flags.gpsc_supported) {
 		if (fcport->id_changed) {
 			fcport->id_changed = 0;
diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index 87c9404aa401..7450c3458be7 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -37,11 +37,6 @@ int qla_nvme_register_remote(struct scsi_qla_host *vha, struct fc_port *fcport)
 		(fcport->nvme_flag & NVME_FLAG_REGISTERED))
 		return 0;
 
-	if (atomic_read(&fcport->state) == FCS_ONLINE)
-		return 0;
-
-	qla2x00_set_fcport_state(fcport, FCS_ONLINE);
-
 	fcport->nvme_flag &= ~NVME_FLAG_RESETTING;
 
 	memset(&req, 0, sizeof(struct nvme_fc_port_info));
-- 
2.35.3

Re: [PATCH v1] qla2xxx: Allow nvme report port registration

[Daniel Wagner]

On Thu, Jul 28, 2022 at 01:50:07PM +0200, Daniel Wagner wrote:
> Move the both ONLINE state check into qla2x00_update_fcport and call
> both register port register functions.
> 
> Currently, qla2x00_reg_remote_port and qla_nvme_register_remote check
> the state if it is ONLINE. If it not, the state is set to ONLINE and
> the function is executed.
> 
> qla2x00_reg_remote_port is called before qla_nvme_register_remote and
> hence qla_nvme_register_remote will always bail out and never register
> a nvme remote port.
> 
> Fixes: 6a45c8e137d4 ("scsi: qla2xxx: Fix disk failure to rediscover")
> Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
> Cc: Quinn Tran <qutran@marvell.com>
> Cc: Nilesh Javali <njavali@marvell.com>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>

I see the offending patch got reverted. FWIW, this patch works for me
fine.

Re: [PATCH v1] qla2xxx: Allow nvme report port registration

[Himanshu Madhani]

Hey Daniel,

> On Jul 28, 2022, at 6:47 AM, Daniel Wagner <dwagner@suse.de> wrote:
> 
> On Thu, Jul 28, 2022 at 01:50:07PM +0200, Daniel Wagner wrote:
>> Move the both ONLINE state check into qla2x00_update_fcport and call
>> both register port register functions.
>> 
>> Currently, qla2x00_reg_remote_port and qla_nvme_register_remote check
>> the state if it is ONLINE. If it not, the state is set to ONLINE and
>> the function is executed.
>> 
>> qla2x00_reg_remote_port is called before qla_nvme_register_remote and
>> hence qla_nvme_register_remote will always bail out and never register
>> a nvme remote port.
>> 
>> Fixes: 6a45c8e137d4 ("scsi: qla2xxx: Fix disk failure to rediscover")
>> Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
>> Cc: Quinn Tran <qutran@marvell.com>
>> Cc: Nilesh Javali <njavali@marvell.com>
>> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> 
> I see the offending patch got reverted. FWIW, this patch works for me
> fine.

Do you see issue with reverted patch? 

I see no issues with the code change per say, just want to understand if
you were seeing issue with the offending patch reverted as well? 

--
Himanshu Madhani	Oracle Linux Engineering

Re: [PATCH v1] qla2xxx: Allow nvme report port registration

[Daniel Wagner]

On Wed, Aug 03, 2022 at 05:30:24PM +0000, Himanshu Madhani wrote:
> Do you see issue with reverted patch?

No, all is good again with 6a45c8e137d4 ("scsi: qla2xxx: Fix disk
failure to rediscover") reverted.

> I see no issues with the code change per say, just want to understand if
> you were seeing issue with the offending patch reverted as well?

I didn't see the revert in time hence I started to fix the code because
I understood the patch actually fixes a race.