Re: [PATCH v2 05/33] ibmvfc: move target list from host to protocol specific channel groups

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing memory leak and mempool_destroy BUG() trigger during module teardown for orphaned targets
--

--- Patch [5]: [PATCH v2 05/33] ibmvfc: move target list from host to protocol specific channel groups ---
commit daeeaeede0866c302e3a906a87d7eef6faf608ae
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvfc: move target list from host to protocol specific channel groups

This commit prepares the driver for protocol-specific target management by
moving the target list and target count out of struct ibmvfc_host and into
struct ibmvfc_channels. The existing target iteration, allocation, and
discovery paths are updated to use the SCSI channel group's target list.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 412e946abcb77..78b4ac201a494 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
[ ... ]
> @@ -5662,7 +5662,7 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)
>  		break;
>  	case IBMVFC_HOST_ACTION_TGT_DEL:
>  	case IBMVFC_HOST_ACTION_TGT_DEL_FAILED:
> -		list_for_each_entry(tgt, &vhost->targets, queue) {
> +		list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {
>  			if (tgt->action == IBMVFC_TGT_ACTION_LOGOUT_RPORT) {
>  				tgt->job_step(tgt);
>  				break;
> @@ -5674,7 +5674,7 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)
>  			return;
>  		}
>  
> -		list_for_each_entry(tgt, &vhost->targets, queue) {
> +		list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {

[Severity: High]
This is a pre-existing issue, but does this code leave targets orphaned on the
new vhost->scsi_scrqs.targets list during module teardown?

If a target is in IBMVFC_TGT_ACTION_DEL_AND_LOGOUT_RPORT when
ibmvfc_remove() begins, ibmvfc_link_down() fails to change its state.

The worker thread in ibmvfc_do_work() transitions the target to
IBMVFC_TGT_ACTION_LOGOUT_DELETED_RPORT without unlinking it:

drivers/scsi/ibmvscsi/ibmvfc.c:ibmvfc_do_work() {
    ...
    ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_LOGOUT_DELETED_RPORT);
    ...
}

It then calls fc_remote_port_delete(), which schedules asynchronous teardown.

The worker thread sets vhost->action = IBMVFC_HOST_ACTION_NONE and
wakes the teardown wait queue. ibmvfc_remove() resumes and permanently kills
the worker thread before the target completes its asynchronous teardown:

drivers/scsi/ibmvscsi/ibmvfc.c:ibmvfc_remove() {
    ...
    kthread_stop(vhost->work_thread);
    ...
}

When the asynchronous terminate_rport_io() callback finally fires, it changes
the target state to LOGOUT_RPORT, but the dead worker thread cannot process it.

Does this leave the orphaned target on vhost->scsi_scrqs.targets indefinitely,
preventing its memory from being freed and triggering a
BUG_ON(pool->curr_nr != pool->min_nr) during mempool_destroy() in
ibmvfc_free_mem()?

drivers/scsi/ibmvscsi/ibmvfc.c:ibmvfc_free_mem() {
    ...
    mempool_destroy(vhost->tgt_pool);
    ...
}

>  			if (tgt->action == IBMVFC_TGT_ACTION_DEL_RPORT) {
>  				tgt_dbg(tgt, "Deleting rport\n");
>  				rport = tgt->rport;

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