From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] pci/uio: enable prefetchable resources mapping Date: Sun, 28 Oct 2018 01:59:12 +0000 Message-ID: <7386777f-9c7f-1370-1b55-cdad197e006a@intel.com> References: <1496530644-8393-1-git-send-email-changpeng.liu@intel.com> <20171005082834.GA20768@bricha3-MOBL3.ger.corp.intel.com> <20171005083351.GB20768@bricha3-MOBL3.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Bruce Richardson , dpdk-dev , Rafal Kozik To: Changpeng Liu Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 865F6239 for ; Sun, 28 Oct 2018 02:59:16 +0100 (CET) In-Reply-To: <20171005083351.GB20768@bricha3-MOBL3.ger.corp.intel.com> Content-Language: en-US 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 10/5/2017 9:33 AM, bruce.richardson at intel.com (Bruce Richardson) wrote: > 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 Hi Changpeng, Following patch looks like superseded this patch, can you please check it. I will update the status of this patch in patchwork. https://patches.dpdk.org/project/dpdk/list/?series=323&state=%2A&archive=both https://patches.dpdk.org/patch/41971/ >> >>> >>>> >>>> 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 */ >>>> >>> >