Re: [PATCH v9 5/9] scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ

[Tyrel Datwyler <tyreld@linux.ibm.com>]

On 9/10/26 7:44 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Missing address-of operator (`&`) when referencing `Scsi_Host`'s `host_lock`, causing a fatal compilation error.
> --
> 
> commit af3cb177b633d7c7bd704888c846393f549edaef
> Author: Dave Marquardt <davemarq@linux.ibm.com>
> Date:   Fri Sep 11 02:19:05 2026 +0000
> 
>     scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ
> 
> This commit adds an interrupt routine and a draining function for asynchronous
> sub-CRQ events. It handles disabling and enabling IRQs to process entries
> while holding the queue and host locks.
> 
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> index daec81f48857d..553fca31cb3aa 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> 
> [ ... ]
> 
>> @@ -4376,6 +4376,60 @@ static struct ibmvfc_crq *ibmvfc_next_scrq(struct ibmvfc_queue *scrq)
>>  	return crq;
>>  }
>>  
>> +static void ibmvfc_drain_async_subq(struct ibmvfc_queue *scrq)
>> +{
>> +	struct ibmvfc_host *vhost = scrq->vhost;
>> +	unsigned long flags;
>> +	struct ibmvfc_crq *crq;
>> +	int done = 0;
>> +
>> +	spin_lock_irqsave(vhost->host->host_lock, flags);
> 
> [Severity: Low]
> Does this code need an address-of operator for host_lock?
> 
> Since kernel 5.14, the Scsi_Host's host_lock field is an embedded spinlock_t
> struct, so passing it directly to spin_lock_irqsave causes it to be passed
> by value. This regression will result in a compilation error because the
> lock function expects a pointer to a spinlock_t.
> 
> Would it make sense to also update ibmvfc_process_async_work, as well as
> ibmvfc_async_fpin_test and ibmvfc_full_fpin_test in the KUnit tests, which
> appear to have this same pattern introduced earlier in the series?
> 
>> +	spin_lock(scrq->q_lock);
>> +	while (!done) {
> 
> [ ... ]
> 
>> +		} else
>> +			done = 1;
>> +	}
>> +	spin_unlock(scrq->q_lock);
>> +	spin_unlock_irqrestore(vhost->host->host_lock, flags);
> 
> [Severity: Low]
> Similar to the lock acquisition above, does spin_unlock_irqrestore also
> need an address-of operator to pass a pointer to host_lock?
> 
>> +}
> 

Again, as noted in patch 1 this breaks the build. Will fix in v10.

-Tyrel