From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 72F55A0C43;
	Thu, 19 Aug 2021 16:52:40 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 0826E4013F;
	Thu, 19 Aug 2021 16:52:40 +0200 (CEST)
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
 by mails.dpdk.org (Postfix) with ESMTP id 5775B4003D
 for <dev@dpdk.org>; Thu, 19 Aug 2021 16:52:39 +0200 (CEST)
X-IronPort-AV: E=McAfee;i="6200,9189,10081"; a="216298328"
X-IronPort-AV: E=Sophos;i="5.84,335,1620716400"; d="scan'208";a="216298328"
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
 by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 19 Aug 2021 07:52:38 -0700
X-IronPort-AV: E=Sophos;i="5.84,335,1620716400"; d="scan'208";a="594604336"
Received: from bricha3-mobl.ger.corp.intel.com ([10.252.21.121])
 by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA;
 19 Aug 2021 07:52:34 -0700
Date: Thu, 19 Aug 2021 15:52:31 +0100
From: Bruce Richardson <bruce.richardson@intel.com>
To: Chengwen Feng <fengchengwen@huawei.com>
Cc: thomas@monjalon.net, ferruh.yigit@intel.com, jerinj@marvell.com,
 jerinjacobk@gmail.com, andrew.rybchenko@oktetlabs.ru, dev@dpdk.org,
 mb@smartsharesystems.com, nipun.gupta@nxp.com,
 hemant.agrawal@nxp.com, maxime.coquelin@redhat.com,
 honnappa.nagarahalli@arm.com, david.marchand@redhat.com,
 sburla@marvell.com, pkapoor@marvell.com, konstantin.ananyev@intel.com
Message-ID: <YR5wLw7C3t+IuY8G@bricha3-MOBL.ger.corp.intel.com>
References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com>
 <1628845774-48339-1-git-send-email-fengchengwen@huawei.com>
 <1628845774-48339-2-git-send-email-fengchengwen@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1628845774-48339-2-git-send-email-fengchengwen@huawei.com>
Subject: Re: [dpdk-dev] [PATCH v15 1/6] dmadev: introduce DMA device library
 public APIs
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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;
  }