From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 77154A0C47; Thu, 2 Sep 2021 10:06:55 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F38F940041; Thu, 2 Sep 2021 10:06:54 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id DB1144003C for ; Thu, 2 Sep 2021 10:06:52 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10094"; a="282734648" X-IronPort-AV: E=Sophos;i="5.84,371,1620716400"; d="scan'208";a="282734648" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Sep 2021 01:06:32 -0700 X-IronPort-AV: E=Sophos;i="5.84,371,1620716400"; d="scan'208";a="691257883" Received: from mloughma-mobl.ger.corp.intel.com (HELO bricha3-MOBL.ger.corp.intel.com) ([10.252.1.171]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 02 Sep 2021 01:06:30 -0700 Date: Thu, 2 Sep 2021 09:06:26 +0100 From: Bruce Richardson To: Jerin Jacob Cc: dpdk-dev , "Walsh, Conor" , "Laatz, Kevin" , fengchengwen , Jerin Jacob , Satananda Burla Message-ID: References: <20210826183301.333442-1-bruce.richardson@intel.com> <20210901163216.120087-1-bruce.richardson@intel.com> <20210901163216.120087-4-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v2 3/6] app/test: add basic dmadev copy tests X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Sep 02, 2021 at 01:14:38PM +0530, Jerin Jacob wrote: > On Wed, Sep 1, 2021 at 10:02 PM Bruce Richardson > wrote: > > > > For each dmadev instance, perform some basic copy tests to validate that > > functionality. > > > > Signed-off-by: Bruce Richardson > > --- > > app/test/test_dmadev.c | 174 +++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 174 insertions(+) > > > > diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c > > index 12f7c69629..261f45db71 100644 > > --- a/app/test/test_dmadev.c > > +++ b/app/test/test_dmadev.c > > @@ -2,12 +2,15 @@ > > * Copyright(c) 2021 HiSilicon Limited. > > * Copyright(c) 2021 Intel Corporation. > > */ > > +#include > > #include > > > > #include > > #include > > #include > > #include > > +#include > > +#include > > > > #include "test.h" > > > > @@ -16,6 +19,11 @@ extern int test_dmadev_api(uint16_t dev_id); > > > > #define PRINT_ERR(...) print_err(__func__, __LINE__, __VA_ARGS__) > > > > +#define COPY_LEN 1024 > > + > > +static struct rte_mempool *pool; > > +static uint16_t id_count; > > + > > static inline int > > __rte_format_printf(3, 4) > > print_err(const char *func, int lineno, const char *format, ...) > > @@ -31,6 +39,134 @@ print_err(const char *func, int lineno, const char *format, ...) > > return ret; > > } > > > > +static inline void > > +await_hw(int dev_id, uint16_t vchan) > > +{ > > + int idle = rte_dmadev_vchan_idle(dev_id, vchan); > > + if (idle < 0) { > > + /* for drivers that don't support this op, just sleep for 25 microseconds */ > > + usleep(25); > > + return; > > + } > > Can following model eliminate the need for rte_dmadev_vchan_idle() API. Right? > > static inline bool > await_hw(int dev_id, uint16_t vchan, uint16_t nb_req, uint16_t *last_idx) > { > const uint64_t tmo = rte_get_timer_hz(); > bool has_error = false; > > const uint64_t end_cycles = rte_get_timer_cycles() + tmo; > while (rte_get_timer_cycles() < end_cycles && nb_req > 0 > && has_error == false) { > rte_pause(); > nb_req -= rte_dmadev_completed(dev_id, > nb_req, last_idx, &has_error); > } > > return has_error ; > } > It would, but unfortunately it also removes the possibility of doing a number of the tests in the set, particularly around failure handling. We used runtime coverage tools to ensure we were hitting as many legs of code as possible in drivers, and to cover these possibilities we need to do various different types of completion gathering, e.g. gather multiple bursts in one go, gathering a single burst in two halves, gathering a burst using completion_status rather than completion, gathering completions one-at-a-time with a call for each individually, and for error handling gathering just the failing element alone, or gathering completions for all remaining elements not just the failing one, etc. etc. These tests are useful both for finding bugs (and they did find ones in our drivers), but also to ensure similar behaviour across different drivers using the API. However, they really only can be done in a consistent way if we are able to ensure that at certain points the hardware has finished processing before we begin gathering completions. Therefore, having a way to poll for idle is useful. As you see, I've also left in the delay as a fallback in case drivers choose not to implement it, thereby making it an optional API. Beyond testing, I can see the API to poll for idleness being useful for the device shutdown case. I was considering whether the "stop" API should also use it to ensure that the hardware is idle before stopping. I decided against it for now, but you could see applications making use of this - waiting for the hardware to finish its work before stopping it. Regards, /Bruce