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

[Tyrel Datwyler]

On 7/30/26 2:28 PM, Bart Van Assche wrote:
> 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.
> 
First off rport is an fc_port in this context not an nvme_rport (whoops), and
second we don't need to check for its existence at all here as we will get it
from the tgt in the else body and check it there. That check got copied out of
the add target code where we check if a target we are adding has suddenly moved
to being deleted.

-Tyrel