Re: [PATCH v2 5/7] ibmvfc: allocate asynchronous sub-queue

[Dave Marquardt <davemarq@linux.ibm.com>]

Tyrel Datwyler <tyreld@linux.ibm.com> writes:

> On 6/8/26 11:30 AM, Dave Marquardt via B4 Relay wrote:
>> From: Dave Marquardt <davemarq@linux.ibm.com>
>> 
>> Allocate and set up the asynchronous sub-queue for asynchronous
>> events, as required for full and extended FPIN support.
>> ---
>>  drivers/scsi/ibmvscsi/ibmvfc.c | 28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>> 
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
>> index a18861808325..ad1f5636e879 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
>> @@ -5352,6 +5352,8 @@ static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
>>  			for (i = 0; i < active_queues; i++)
>>  				scrqs->scrqs[i].vios_cookie =
>>  					be64_to_cpu(setup->channel_handles[i]);
>> +			scrqs->async_scrq->vios_cookie =
>> +				be64_to_cpu(setup->asyncSubqHandle);
>>  
>>  			ibmvfc_dbg(vhost, "Using %u channels\n",
>>  				   vhost->scsi_scrqs.active_queues);
>> @@ -5402,6 +5404,7 @@ static void ibmvfc_channel_setup(struct ibmvfc_host *vhost)
>>  		setup_buf->num_scsi_subq_channels = cpu_to_be32(num_channels);
>>  		for (i = 0; i < num_channels; i++)
>>  			setup_buf->channel_handles[i] = cpu_to_be64(scrqs->scrqs[i].cookie);
>> +		setup_buf->asyncSubqHandle = cpu_to_be64(scrqs->async_scrq->cookie);
>>  	}
>>  
>>  	ibmvfc_init_event(evt, ibmvfc_channel_setup_done, IBMVFC_MAD_FORMAT);
>> @@ -6369,6 +6372,24 @@ static int ibmvfc_alloc_channels(struct ibmvfc_host *vhost,
>>  	if (!channels->scrqs)
>>  		return -ENOMEM;
>>  
>> +	channels->async_scrq = kzalloc_obj(*channels->async_scrq, GFP_KERNEL);
>> +
>> +	if (!channels->async_scrq) {
>> +		kfree(channels->scrqs);
>> +		channels->scrqs = NULL;
>> +		return -ENOMEM;
>
> This failure cleanup code starts duplicating here.
>
>> +	}
>> +
>> +	rc = ibmvfc_alloc_queue(vhost, channels->async_scrq,
>> +				IBMVFC_SUB_CRQ_FMT);
>> +	if (rc) {
>> +		kfree(channels->scrqs);
>> +		channels->scrqs = NULL;
>> +		kfree(channels->async_scrq);
>> +		channels->async_scrq = NULL;
>
> Again here plus freeing channels->scrqs memory.
>
>> +		return rc;
>> +	}
>> +
>>  	for (i = 0; i < channels->max_queues; i++) {
>>  		scrq = &channels->scrqs[i];
>>  		rc = ibmvfc_alloc_queue(vhost, scrq, IBMVFC_SUB_CRQ_FMT);
>> @@ -6380,6 +6401,9 @@ static int ibmvfc_alloc_channels(struct ibmvfc_host *vhost,
>>  			kfree(channels->scrqs);
>>  			channels->scrqs = NULL;
>>  			channels->active_queues = 0;
>> +			ibmvfc_free_queue(vhost, channels->async_scrq);
>> +			kfree(channels->async_scrq);
>> +			channels->async_scrq = NULL;
>
> And then again here. Could use goto to do the frees at the end of the function.
>
> free_async:
> 	kfree(channels->async_scrq);
> 	channels->async = NULL;
> free_scrqs:
> 	kfree(channels->scrqs);
> 	channels->scrqs = NULL;
>
> return rc;
>
>>  			return rc;
>>  		}
>>  	}
>> @@ -6418,6 +6442,10 @@ static void ibmvfc_release_channels(struct ibmvfc_host *vhost,
>>  
>>  		kfree(channels->scrqs);
>>  		channels->scrqs = NULL;
>> +
>> +		ibmvfc_free_queue(vhost, channels->async_scrq);
>
> Looks like missing kfree(channels->async_scrq) here.

I'll clean this up. Thanks.

-Dave