Re: [PATCH v2 25/33] ibmvfc: process NVMe/FC rports in work thread

[Bart Van Assche]

On 7/29/26 11:52 PM, Nathan Chancellor wrote:
> Hi Tyrel,
>
> On Wed, Jul 22, 2026 at 05:01:41PM -0700, Tyrel Datwyler wrote:
> > diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> > index 81d9229bf388..ffb579816e84 100644
> > --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> > +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> > @@ -6073,6 +6119,30 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)
> >   			}
> >   		}
> >   
> > +		list_for_each_entry(tgt, &vhost->nvme_scrqs.targets, queue) {
> > +			if (tgt->action == IBMVFC_TGT_ACTION_DEL_RPORT) {
> > +				tgt_dbg(tgt, "Deleteing NVMe rport\n");
> > +				nvme_rport = tgt->nvme_remote_port;
> > +				list_del(&tgt->queue);
> > +				ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_DELETED_RPORT);
> > +				spin_unlock_irqrestore(vhost->host->host_lock, flags);
> > +				if (nvme_rport)
> > +					ibmvfc_nvme_unregister_remoteport(tgt);
> > +				timer_delete_sync(&tgt->timer);
> > +				kref_put(&tgt->kref, ibmvfc_release_tgt);
> > +				return;
> > +			} else if (rport && tgt->action == IBMVFC_TGT_ACTION_DEL_AND_LOGOUT_RPORT) {
> > +				tgt_dbg(tgt, "Deleting NVMe rport with outstanding I/O\n");
> > +				nvme_rport = tgt->nvme_remote_port;
> > +				ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_LOGOUT_DELETED_RPORT);
> > +				tgt->init_retries = 0;
> > +				spin_unlock_irqrestore(vhost->host->host_lock, flags);
> > +				if (nvme_rport)
> > +					ibmvfc_nvme_unregister_remoteport(tgt);
> > +				return;
> > +			}
> > +		}
> > +
> >   		if (vhost->state == IBMVFC_INITIALIZING) {
> >   			if (vhost->action == IBMVFC_HOST_ACTION_TGT_DEL_FAILED) {
> >   				if (vhost->reinit) {
>
> There is a warning from clang after this change landed in -next as
> commit 696d1cc2aaa2 ("scsi: ibmvfc: process NVMe/FC rports in work
> thread"), breaking the build when -Werror is enabled:
>
>    drivers/scsi/ibmvscsi/ibmvfc-core.c:6154:15: error: variable 'rport' is uninitialized when used here [-Werror,-Wuninitialized]
>     6154 |                         } else if (rport && tgt->action == IBMVFC_TGT_ACTION_DEL_AND_LOGOUT_RPORT) {
>          |                                    ^~~~~
>
> Sashiko appears to point this out as well:
>
>    https://lore.kernel.org/20260723005006.C7EE21F00A3A@xxxxxxxxxxxxxxx/
>
> But I am not sure that its suggestion to use nvme_rport is correct given
> the context of this code, hence just the report from my end.

I think that advice is wrong because nvme_rport has not yet been set if
the "else" statement is reached. How about the change below?

Thanks,

Bart.

diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c 
b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index dfa231b6cc47..9e752978dad8 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -5984,8 +5984,6 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)
 {
 	struct ibmvfc_target *tgt;
 	unsigned long flags;
-	struct fc_rport *rport;
-	struct nvme_fc_remote_port *nvme_rport;
 	LIST_HEAD(purge);
 	int rc;

@@ -6103,6 +6101,8 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)

 		list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {
 			if (tgt->action == IBMVFC_TGT_ACTION_DEL_RPORT) {
+				struct fc_rport *rport;
+
 				tgt_dbg(tgt, "Deleting rport\n");
 				rport = tgt->rport;
 				tgt->rport = NULL;
@@ -6115,6 +6115,8 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)
 				kref_put(&tgt->kref, ibmvfc_release_tgt);
 				return;
 			} else if (tgt->action == IBMVFC_TGT_ACTION_DEL_AND_LOGOUT_RPORT) {
+				struct fc_rport *rport;
+
 				tgt_dbg(tgt, "Deleting rport with I/O outstanding\n");
 				rport = tgt->rport;
 				tgt->rport = NULL;
@@ -6141,6 +6143,8 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)

 		list_for_each_entry(tgt, &vhost->nvme_scrqs.targets, queue) {
 			if (tgt->action == IBMVFC_TGT_ACTION_DEL_RPORT) {
+				struct nvme_fc_remote_port *nvme_rport;
+
 				tgt_dbg(tgt, "Deleteing NVMe rport\n");
 				nvme_rport = tgt->nvme_remote_port;
 				list_del(&tgt->queue);
@@ -6151,7 +6155,9 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)
 				timer_delete_sync(&tgt->timer);
 				kref_put(&tgt->kref, ibmvfc_release_tgt);
 				return;
-			} else if (rport && tgt->action == 
IBMVFC_TGT_ACTION_DEL_AND_LOGOUT_RPORT) {
+			} else if (tgt->action == IBMVFC_TGT_ACTION_DEL_AND_LOGOUT_RPORT) {
+				struct nvme_fc_remote_port *nvme_rport;
+
 				tgt_dbg(tgt, "Deleting NVMe rport with outstanding I/O\n");
 				nvme_rport = tgt->nvme_remote_port;
 				ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_LOGOUT_DELETED_RPORT);