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

[Bruce Richardson <bruce.richardson@intel.com>]

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 <changpeng.liu@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 */
> > > 
> >