On Sat, Sep 04, 2021 at 06:10:22PM +0800, Chengwen Feng wrote:

This patch introduce DMA device library internal header, which contains
internal data types that are used by the DMA devices in order to expose
their ops to the class.

Signed-off-by: Chengwen Feng <redacted>
Acked-by: Bruce Richardson <redacted>
Acked-by: Morten Brørup <redacted>
Reviewed-by: Kevin Laatz <redacted>
Reviewed-by: Conor Walsh <redacted>
---

<snip>

+struct rte_dmadev {
+	void *dev_private;
+	/**< PMD-specific private data.
+	 *
+	 * - If is the primary process, after dmadev allocated by
+	 * rte_dmadev_pmd_allocate(), the PCI/SoC device probing should
+	 * initialize this field, and copy it's value to the 'dev_private'
+	 * field of 'struct rte_dmadev_data' which pointer by 'data' filed.
+	 *
+	 * - If is the secondary process, dmadev framework will initialize this
+	 * field by copy from 'dev_private' field of 'struct rte_dmadev_data'
+	 * which initialized by primary process.
+	 *
+	 * @note It's the primary process responsibility to deinitialize this
+	 * field after invoke rte_dmadev_pmd_release() in the PCI/SoC device
+	 * removing stage.
+	 */
+	rte_dmadev_copy_t             copy;
+	rte_dmadev_copy_sg_t          copy_sg;
+	rte_dmadev_fill_t             fill;
+	rte_dmadev_submit_t           submit;
+	rte_dmadev_completed_t        completed;
+	rte_dmadev_completed_status_t completed_status;
+	void *reserved_ptr[7]; /**< Reserved for future IO function. */

This is new in this set, I think. I assume that 7 was chosen so that we
have the "data" pointer and the "dev_ops" pointers on the second cacheline
(if 64-byte CLs)? If so, I wonder if we can find a good way to express that
in the code or in the comments?

The simplest - and probably as clear as any - is to split this into
"void *__reserved_cl0" and "void *__reserved_cl1[6]" to show that it is
split across the two cachelines, with the latter having comment:
"Reserve space for future IO functions, while keeping data and dev_ops
pointers on the second cacheline"

If we don't mind using a slightly different type the magic "6" could be
changed to a computation:
char __reserved_cl1[RTE_CACHELINE_SZ - sizeof(void *) * 2];

However, for simplicity, I think the magic 6 can be kept, and just split
into reserved_cl0 and reserved_cl1 as I suggest above.

/Bruce