Re: [PATCH 13/29] ibmvfc: add NVMe/FC Port Login support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Targets allocated from a mempool are incorrectly freed using `kfree()` directly, permanently depleting the mempool's reserved elements.
--

commit 6a208595368e371e1b9ab8d0fccb04902f87daf8
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvfc: add NVMe/FC Port Login support
    
Expand the target login path to issue the NVMe/FC-specific port login
MAD for NVMe targets. Select the correct PLOGI MAD opcode based on the
target protocol and include the protocol name in success and failure
logging.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 3e3d77e0d5172..2c54d0b9add49 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c

[ ... ]

> @@ -4286,7 +4286,10 @@ static void ibmvfc_tgt_send_plogi(struct ibmvfc_target *tgt)
>  	} else {
>  		plogi->common.version = cpu_to_be32(1);
>  	}
> -	plogi->common.opcode = cpu_to_be32(IBMVFC_PORT_LOGIN);
> +	if (tgt->protocol == IBMVFC_PROTO_SCSI)
> +		plogi->common.opcode = cpu_to_be32(IBMVFC_PORT_LOGIN);
> +	else
> +		plogi->common.opcode = cpu_to_be32(IBMVFC_NVMF_PORT_LOGIN);
>  	plogi->common.length = cpu_to_be16(sizeof(*plogi));
>  	plogi->scsi_id = cpu_to_be64(tgt->scsi_id);
>  
> @@ -4295,7 +4298,7 @@ static void ibmvfc_tgt_send_plogi(struct ibmvfc_target *tgt)
>  		ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_NONE);
>  		kref_put(&tgt->kref, ibmvfc_release_tgt);

[Severity: High]
This isn't a bug introduced by this patch, but does the target release
function deplete the mempool's reserved elements during memory pressure?

The target structure is allocated from the driver's mempool:

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_alloc_target() {
    ...
    tgt = mempool_alloc(vhost->tgt_pool, GFP_NOIO);
    ...
}

However, it appears ibmvfc_release_tgt() uses kfree() instead of
mempool_free():

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_release_tgt() {
    struct ibmvfc_target *tgt = container_of(kref, struct ibmvfc_target, kref);
    kfree(tgt);
}

When the system is under memory pressure, mempool_alloc() falls back to
using its pre-allocated reserve elements.

If these elements are returned to the general slab allocator via kfree()
instead of mempool_free(), wouldn't this permanently deplete the mempool's
reserve and cause subsequent allocations under memory pressure to hang
indefinitely?

>  	} else
> -		tgt_dbg(tgt, "Sent port login\n");
> +		tgt_dbg(tgt, "Sent %s port login\n", proto_type[tgt->protocol]);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623013035.3436640-1-tyreld@linux.ibm.com?part=13