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

[Bruce Richardson]

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 at huawei.com>
> Acked-by: Bruce Richardson <bruce.richardson at intel.com>
> Acked-by: Morten Brørup <mb at smartsharesystems.com>
> Acked-by: Jerin Jacob <jerinjacobk at 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;
  }