Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping

["Liu, Changpeng" <changpeng.liu@intel.com>]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Sunday, October 28, 2018 9:59 AM
> To: Liu, Changpeng <changpeng.liu@intel.com>
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; dpdk-dev
> <dev@dpdk.org>; Rafal Kozik <rk@semihalf.com>
> Subject: Re: [dpdk-dev] [PATCH] pci/uio: enable prefetchable resources mapping
> 
> 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=bot
> h
> https://patches.dpdk.org/patch/41971/
Yeah, this feature has been enabled with DPDK now, you can update the patch status from me.
> 
> 
> >>
> >>>
> >>>>
> >>>> Signed-off-by: Changpeng Liu <changpeng.liu at intel.com>
> >>>> ---
> >>>>  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 */
> >>>>
> >>>
> >