[dpdk-dev] [PATCH v20 2/7] dmadev: introduce DMA device library internal header

[Bruce Richardson]

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 <fengchengwen at huawei.com>
> Acked-by: Bruce Richardson <bruce.richardson at intel.com>
> Acked-by: Morten Brørup <mb at smartsharesystems.com>
> Reviewed-by: Kevin Laatz <kevin.laatz at intel.com>
> Reviewed-by: Conor Walsh <conor.walsh at intel.com>
> ---
<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