Already fix in V21, thanks
On 2021/9/6 21:35, Bruce Richardson wrote:
> 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
>
> .
>
