On 20.01.25 11:07, Julian Ruess wrote: > On Mon Jan 20, 2025 at 10:56 AM CET, Alexandra Winter wrote: >> >> >> 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. > > That is true for the IBM ISM vPCI device but maybe we > should design the API also for future PCI devices > that do not move data synchronously. > An API should always be extendable >> >> 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); >>>> -- >
