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

[fengchengwen <fengchengwen@huawei.com>]

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