Re: [PATCH v2 1/7] ibmvfc: add basic FPIN support

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

On 6/15/26 1:37 PM, Dave Marquardt wrote:
> 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>
>>>
>>> Add support for basic FPIN messages to the ibmvfc driver. This includes
>>>
>>> - adding FPIN handling support to the async event handler
>>> - offloading processing of FPIN messages to a work queue
>>> - converting the VIOS FPIN message to a struct fc_els_fpin as used by
>>>   the Linux kernel
>>> - passing the converted struct fc_els_fpin to fc_host_fpin_rcv for
>>>   processing
>>>

<snip>

>>> +static void ibmvfc_process_async_work(struct work_struct *work)
>>> +{
>>> +	struct ibmvfc_async_work *aw;
>>> +	struct ibmvfc_async_crq *crq;
>>> +	struct ibmvfc_target *tgt;
>>> +	struct ibmvfc_host *vhost;
>>> +	struct fc_els_fpin *fpin;
>>> +
>>> +	aw = container_of(work, struct ibmvfc_async_work, async_work_s);
>>> +	crq = aw->crq;
>>> +	vhost = aw->vhost;
>>> +
>>> +	if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
>>> +		goto end;
>>> +	list_for_each_entry(tgt, &vhost->targets, queue) {
>>
>> Should we be holding the host lock when iterateing targets?
> 
> fc_host_fpin_rcv assumes no locks are held. I built a kernel with
> LOCKDEP, PROVE_LOCKING, DEBUG_SPINLOCK et al defined, and got some
> deadlocks. Some of the routines fc_host_fpin_rcv call acquire the host
> lock also.
> 
> I'l change this code to use list_for_each_entry_safe. Is that enough for
> safety in this case?
> 

No, the _safe version is only if you are potentially doing removals in the code
block while iterating the list.

Can't we assume every target in our tgt list is unique and we only need to
iterate until we find the matching target? In that case you can take the lock
iterate the list, break on a found match, copy whatever data is necessary from
the tgt, and then drop the lock and call fc_host_fpin_rcv.

-Tyrel