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 155EBA0547; Wed, 29 Sep 2021 14:17:28 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 76C49410E5; Wed, 29 Sep 2021 14:17:27 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 442A740685 for ; Wed, 29 Sep 2021 14:17:25 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10121"; a="285941152" X-IronPort-AV: E=Sophos;i="5.85,332,1624345200"; d="scan'208";a="285941152" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Sep 2021 05:17:24 -0700 X-IronPort-AV: E=Sophos;i="5.85,332,1624345200"; d="scan'208";a="707240985" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.20.220]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 29 Sep 2021 05:17:22 -0700 Date: Wed, 29 Sep 2021 13:17:18 +0100 From: Bruce Richardson To: "Ananyev, Konstantin" Cc: "Xueming(Steven) Li" , "jerinjacobk@gmail.com" , NBU-Contact-Thomas Monjalon , "andrew.rybchenko@oktetlabs.ru" , "dev@dpdk.org" , "Yigit, Ferruh" Message-ID: References: <543d0e4ac61633fa179506906f88092a6d928fe6.camel@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue 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 Wed, Sep 29, 2021 at 12:46:51PM +0100, Ananyev, Konstantin wrote: > > > > -----Original Message----- > > From: Richardson, Bruce > > Sent: Wednesday, September 29, 2021 12:08 PM > > To: Ananyev, Konstantin > > Cc: Xueming(Steven) Li ; jerinjacobk@gmail.com; NBU-Contact-Thomas Monjalon ; > > andrew.rybchenko@oktetlabs.ru; dev@dpdk.org; Yigit, Ferruh > > Subject: Re: [dpdk-dev] [PATCH v1] ethdev: introduce shared Rx queue > > > > On Wed, Sep 29, 2021 at 09:52:20AM +0000, Ananyev, Konstantin wrote: > > > > > > > > > > -----Original Message----- > > > > From: Xueming(Steven) Li > > > > Sent: Wednesday, September 29, 2021 10:13 AM > > > > > > > + /* Locate real source fs according to mbuf->port. */ > > > > > + for (i = 0; i < nb_rx; ++i) { > > > > > + rte_prefetch0(pkts_burst[i + 1]); > > > > > > > > > > you access pkt_burst[] beyond array boundaries, > > > > > also you ask cpu to prefetch some unknown and possibly invalid > > > > > address. > > > > > > > > Sorry I forgot this topic. It's too late to prefetch current packet, so > > > > perfetch next is better. Prefetch an invalid address at end of a look > > > > doesn't hurt, it's common in DPDK. > > > > > > First of all it is usually never 'OK' to access array beyond its bounds. > > > Second prefetching invalid address *does* hurt performance badly on many CPUs > > > (TLB misses, consumed memory bandwidth etc.). > > > As a reference: https://lwn.net/Articles/444346/ > > > If some existing DPDK code really does that - then I believe it is an issue and has to be addressed. > > > More important - it is really bad attitude to submit bogus code to DPDK community > > > and pretend that it is 'OK'. > > > > > > > The main point we need to take from all this is that when > > prefetching you need to measure perf impact of it. > > > > In terms of the specific case of prefetching one past the end of the array, > > I would take the view that this is harmless in almost all cases. Unlike any > > prefetch of "NULL" as in the referenced mail, reading one past the end (or > > other small number of elements past the end) is far less likely to cause a > > TLB miss - and it's basically just reproducing behaviour we would expect > > off a HW prefetcher (though those my explicitly never cross page > > boundaries). However, if you feel it's just cleaner to put in an > > additional condition to remove the prefetch for the end case, that's ok > > also - again so long as it doesn't affect performance. [Since prefetch is a > > hint, I'm not sure if compilers or CPUs may be legally allowed to skip the > > branch and blindly prefetch in all cases?] > > Please look at the code. > It doesn't prefetch next element beyond array boundaries. > It first reads address from the element that is beyond array boundaries (which is a bug by itself). > Then it prefetches that bogus address. > We simply don't know is this address is valid and where it points to. > > In other words, it doesn't do: > rte_prefetch0(&pkts_burst[i + 1]); > > It does: > rte_prefetch0(pkts_burst[i + 1]); > Apologies, yes, you are right, and that is a bug. /Bruce