From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 8BFCC1B1B0 for ; Thu, 5 Oct 2017 10:33:55 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga105.jf.intel.com with ESMTP; 05 Oct 2017 01:33:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,480,1500966000"; d="scan'208";a="159667420" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.37]) by fmsmga006.fm.intel.com with SMTP; 05 Oct 2017 01:33:52 -0700 Received: by (sSMTP sendmail emulation); Thu, 05 Oct 2017 09:33:51 +0100 Date: Thu, 5 Oct 2017 09:33:51 +0100 From: Bruce Richardson To: Ferruh Yigit Cc: Changpeng Liu , dev@dpdk.org Message-ID: <20171005083351.GB20768@bricha3-MOBL3.ger.corp.intel.com> References: <1496530644-8393-1-git-send-email-changpeng.liu@intel.com> <20171005082834.GA20768@bricha3-MOBL3.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171005082834.GA20768@bricha3-MOBL3.ger.corp.intel.com> Organization: Intel Research and Development Ireland Ltd. User-Agent: Mutt/1.8.3 (2017-05-23) Subject: Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Oct 2017 08:33:55 -0000 On Thu, Oct 05, 2017 at 09:28:34AM +0100, Bruce Richardson wrote: > On Thu, Oct 05, 2017 at 01:06:41AM +0100, Ferruh Yigit wrote: > > On 6/3/2017 11:57 PM, Changpeng Liu wrote: > > > For PCI prefetchable resources, Linux will create a > > > write combined file as well, the library will try > > > to map resourceX_wc file first, if the file does > > > not exist, then it will map resourceX as usual. > > > > Hi Changpeng, > > > > Code part looks OK, but can you please describe more why we should try > > write combined resource file first, what is the benefit of using it _wc > > file? > > > > Thanks, > > ferruh > > > Also, if we use a write combining resource file, I believe we will use s/will use/will lose/ > correct ordering of instructions within our drivers. Does applying this > patch not also mean that we would need memory barriers inside all the > drivers, so as to ensure that we don't have a queue doorbell write > reordered with the descriptor writes? I don't think it's safe to apply > this change on it's own, without driver changes, since all PMDs assume > strong ordering on IA. > > Regards, > /Bruce > > > > > > > > > Signed-off-by: Changpeng Liu > > > --- > > > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 19 ++++++++++++++----- > > > 1 file changed, 14 insertions(+), 5 deletions(-) > > > > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > > > index fa10329..d9fc20a 100644 > > > --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > > > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > > > @@ -321,7 +321,7 @@ > > > > > > /* update devname for mmap */ > > > snprintf(devname, sizeof(devname), > > > - "%s/" PCI_PRI_FMT "/resource%d", > > > + "%s/" PCI_PRI_FMT "/resource%d_wc", > > > pci_get_sysfs_path(), > > > loc->domain, loc->bus, loc->devid, > > > loc->function, res_idx); > > > @@ -335,13 +335,22 @@ > > > } > > > > > > /* > > > - * open resource file, to mmap it > > > + * open prefetchable resource file first, try to mmap it > > > */ > > > fd = open(devname, O_RDWR); > > > if (fd < 0) { > > > - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", > > > - devname, strerror(errno)); > > > - goto error; > > > + snprintf(devname, sizeof(devname), > > > + "%s/" PCI_PRI_FMT "/resource%d", > > > + pci_get_sysfs_path(), > > > + loc->domain, loc->bus, loc->devid, > > > + loc->function, res_idx); > > > + /* then try to map resource file */ > > > + fd = open(devname, O_RDWR); > > > + if (fd < 0) { > > > + RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", > > > + devname, strerror(errno)); > > > + goto error; > > > + } > > > } > > > > > > /* try mapping somewhere close to the end of hugepages */ > > > > >