Re: [dpdk-dev] [PATCH v15 1/6] dmadev: introduce DMA device library public APIs

[fengchengwen <fengchengwen@huawei.com>]

Hi Bruce and All,

The v16 use following define, and it also include dma skeleton and ut.
   struct rte_dmadev_stats {
 	uint64_t submitted;
 	uint64_t completed;
 	uint64_t errors;
   }

Feedback welcome, thanks.

On 2021/8/19 22:52, Bruce Richardson wrote:
> On Fri, Aug 13, 2021 at 05:09:29PM +0800, Chengwen Feng wrote:
>> The 'dmadevice' is a generic type of DMA device.
>>
>> This patch introduce the 'dmadevice' public APIs which expose generic
>> operations that can enable configuration and I/O with the DMA devices.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>> Acked-by: Jerin Jacob <jerinjacobk@gmail.com>
>> ---
> one minor comment for clarification
> 
>> +/**
>> + * rte_dmadev_stats - running statistics.
>> + */
>> +struct rte_dmadev_stats {
>> +	uint64_t submitted_count;
>> +	/**< Count of operations which were submitted to hardware. */
>> +	uint64_t completed_fail_count;
>> +	/**< Count of operations which failed to complete. */
>> +	uint64_t completed_count;
>> +	/**< Count of operations which successfully complete. */
>> +};
> 
> The name of the last variable and the comment on it seem mismatched. The
> name implies that it's all completed ops, i.e. to get successful only you
> do "stats.completed_count - stats.completed_fail_count", while the comment
> says that it's successful only. Therefore I suggest:
> 
> * We rename the last two vars to "completed_fail" and "completed_success"
>   for clarity OR
> * We redefine "completed_count" to be the full completed count of both
>   success and failure.
> 
> I have a slight preference for the latter option, but either can work.
> 
> /Bruce
> 
> PS: We probably don't need "count" on any of these values, based on two
> options above suggest structs as:
> 
>   struct rte_dmadev_stats {
> 	uint64_t submitted;
> 	uint64_t failed;
> 	uint64_t successful;
>   };
> 
> OR:
> 
>   struct rte_dmadev_stats {
> 	uint64_t submitted;
> 	uint64_t completed;
> 	uint64_t errors;
>   }
> .
>