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 <fengcheng...@huawei.com>
> Acked-by: Bruce Richardson <bruce.richard...@intel.com>
> Acked-by: Morten Brørup <m...@smartsharesystems.com>
> Reviewed-by: Kevin Laatz <kevin.la...@intel.com>
> Reviewed-by: Conor Walsh <conor.wa...@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
