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

[Ferruh Yigit <ferruh.yigit@intel.com>]

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 <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 */
>>>>
>>>
>