From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id D3890A0540;
Mon, 13 Jul 2020 11:55:38 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id A0FE21D58B;
Mon, 13 Jul 2020 11:55:37 +0200 (CEST)
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
by dpdk.org (Postfix) with ESMTP id 55BE01D586
for <dev@dpdk.org>; Mon, 13 Jul 2020 11:55:36 +0200 (CEST)
IronPort-SDR: yah/GdEjGRYesaGZhrzsAsm/x7fiPI/Dv+mJr6wcpI1zCbpfAv/SmecW648CK29msNfvKFKQl+
G0kEYFtqiHNg==
X-IronPort-AV: E=McAfee;i="6000,8403,9680"; a="136753101"
X-IronPort-AV: E=Sophos;i="5.75,347,1589266800"; d="scan'208";a="136753101"
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga005.jf.intel.com ([10.7.209.41])
by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
13 Jul 2020 02:55:35 -0700
IronPort-SDR: f/nz3tkuc/SLLY2+1MwlkEWW5irPE+cAbns0EtQfaXhxS9SD+kNJGb0eWW+wD3hNEIIsvMKse/
m9Oi0rUL0Tug==
X-IronPort-AV: E=Sophos;i="5.75,347,1589266800"; d="scan'208";a="459236474"
Received: from bricha3-mobl.ger.corp.intel.com ([10.249.32.149])
by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA;
13 Jul 2020 02:55:33 -0700
Date: Mon, 13 Jul 2020 10:55:30 +0100
From: Bruce Richardson <bruce.richardson@intel.com>
To: Cheng Jiang <Cheng1.jiang@intel.com>
Cc: dev@dpdk.org, patrick.fu@intel.com
Message-ID: <20200713095530.GD694@bricha3-MOBL.ger.corp.intel.com>
References: <20200713071519.110662-1-Cheng1.jiang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200713071519.110662-1-Cheng1.jiang@intel.com>
Subject: Re: [dpdk-dev] [PATCH 20.11] raw/ioat: added a flag to control
copying handle parameters
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
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 Mon, Jul 13, 2020 at 07:15:19AM +0000, Cheng Jiang wrote:
> Added a flag which controls whether rte_ioat_enqueue_copy
> and rte_ioat_completed_copies function should process
> handle parameters to improve the performance when handle
> parameters are not necessary to use. This is targeting
> 20.11 release.
>
> Signed-off-by: Cheng Jiang <Cheng1.jiang@intel.com>
> ---
> drivers/raw/ioat/ioat_rawdev.c | 1 +
> drivers/raw/ioat/rte_ioat_rawdev.h | 14 +++++++++++---
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
<snip>
> @@ -208,6 +213,11 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies,
> if (count > max_copies)
> count = max_copies;
>
> + ioat->next_read = read + count;
> + ioat->completed += count;
> + if (!ioat->hdls_enable)
> + return count;
> +
> for (; i < count - 1; i += 2, read += 2) {
> __m128i hdls0 = _mm_load_si128(&ioat->hdls[read & mask]);
> __m128i hdls1 = _mm_load_si128(&ioat->hdls[(read + 1) & mask]);
> @@ -223,8 +233,6 @@ rte_ioat_completed_copies(int dev_id, uint8_t max_copies,
> dst_hdls[i] = hdls[1];
> }
>
> - ioat->next_read = read;
> - ioat->completed += count;
> return count;
> }
This change I think may cause problems if we ever want to have one thread
enqueuing and another taking completions. The next_read and completed
counters should really only be updated after we have finished reading the
completed handles array. Therefore, for safety, I tihnk it might be better
to keep the updates in their original places and put an "end:" label before
them. Then the "return count" in the middle of the function can be "goto
end;"
Regards,
/Bruce