Re: [RFC net-next 4/7] net/ism: Add kernel-doc comments for ism functions

[Alexandra Winter <wintera@linux.ibm.com>]

On 20.01.25 07:32, Dust Li wrote:
>> +	/**
>> +	 * move_data() - write into a remote dmb
>> +	 * @dev: Local sending ism device
>> +	 * @dmb_tok: Token of the remote dmb
>> +	 * @idx: signalling index
>> +	 * @sf: signalling flag;
>> +	 *      if true, idx will be turned on at target ism interrupt mask
>> +	 *      and target device will be signalled, if required.
>> +	 * @offset: offset within target dmb
>> +	 * @data: pointer to data to be sent
>> +	 * @size: length of data to be sent
>> +	 *
>> +	 * Use dev to write data of size at offset into a remote dmb
>> +	 * identified by dmb_tok. Data is moved synchronously, *data can
>> +	 * be freed when this function returns.
> When considering the API, I found this comment may be incorrect.
> 
> IIUC, in copy mode for PCI ISM devices, the CPU only tells the
> device to perform a DMA copy. As a result, when this function returns,
> the device may not have completed the DMA copy.
> 

No, it is actually one of the properties of ISM vPCI that the data is
moved synchronously inside the move_data() function. (on PCI layer the
data is moved inside the __zpci_store_block() command).
Obviously for loopback move_data() is also synchornous.

SMC-D does not make use of it, instead they re-use the same
conn->sndbuf_desc for the lifetime of a connection.


> In zero-copy mode for loopback, the source and destination share the
> same buffer. If the source rewrites the buffer, the destination may
> encounter corrupted data. The source should only reuse the data after
> the destination has finished reading it.
> 

That is true independent of the question, whether the move is
synchronous or not.
It is the clients' responsibility to make sure a sender does not
overwrite unread data. SMC uses the write-pointers and read-pointer for
that.


> Best regards,
> Dust
> 
>> +	 *
>> +	 * If signalling flag (sf) is true, bit number idx bit will be
>> +	 * turned on in the ism signalling mask, that belongs to the
>> +	 * target dmb, and handle_irq() of the ism client that owns this
>> +	 * dmb will be called, if required. The target device may chose to
>> +	 * coalesce multiple signalling triggers.
>> +	 */
>> 	int (*move_data)(struct ism_dev *dev, u64 dmb_tok, unsigned int idx,
>> 			 bool sf, unsigned int offset, void *data,
>> 			 unsigned int size);
>> --