Re: [dpdk-dev] [RFC] pci: force address of mappings in secondary process

["Tan, Jianfeng" <jianfeng.tan@intel.com>]

> -----Original Message-----
> From: Gonzalez Monroy, Sergio
> Sent: Wednesday, July 12, 2017 3:32 PM
> To: Tan, Jianfeng; Stephen Hemminger; dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC] pci: force address of mappings in secondary
> process
> 
> On 12/07/2017 03:45, Tan, Jianfeng wrote:
> >
> >> -----Original Message-----
> >> From: Gonzalez Monroy, Sergio
> >> Sent: Tuesday, July 11, 2017 7:36 PM
> >> To: Tan, Jianfeng; Stephen Hemminger; dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [RFC] pci: force address of mappings in secondary
> >> process
> >>
> >> On 11/07/2017 02:56, Tan, Jianfeng wrote:
> >>>> -----Original Message-----
> >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
> >>>> Hemminger
> >>>> Sent: Tuesday, July 11, 2017 9:13 AM
> >>>> To: dev@dpdk.org
> >>>> Cc: Stephen Hemminger
> >>>> Subject: [dpdk-dev] [RFC] pci: force address of mappings in secondary
> >>>> process
> >>>>
> >>>> The PCI memory resources in the secondary process should be in
> >>>> the exact same location as the primary process. Otherwise
> >>>> there is a risk of a stray pointer.
> >>>>
> >>>> Not sure if this is right, but it looks like a potential
> >>>> problem.
> >>>>
> >>>> ---
> >>>>    lib/librte_eal/common/eal_common_pci_uio.c | 2 +-
> >>>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/lib/librte_eal/common/eal_common_pci_uio.c
> >>>> b/lib/librte_eal/common/eal_common_pci_uio.c
> >>>> index 367a6816dcb8..2156b1a436c4 100644
> >>>> --- a/lib/librte_eal/common/eal_common_pci_uio.c
> >>>> +++ b/lib/librte_eal/common/eal_common_pci_uio.c
> >>>> @@ -77,7 +77,7 @@ pci_uio_map_secondary(struct rte_pci_device
> *dev)
> >>>>
> >>>>    			void *mapaddr = pci_map_resource(uio_res-
> >>>>> maps[i].addr,
> >>>>    					fd, (off_t)uio_res->maps[i].offset,
> >>>> -					(size_t)uio_res->maps[i].size, 0);
> >>>> +					(size_t)uio_res->maps[i].size,
> >>>> MAP_FIXED);
> >>>>    			/* fd is not needed in slave process, close it */
> >>>>    			close(fd);
> >>>>    			if (mapaddr != uio_res->maps[i].addr) {
> >>>> --
> >>>> 2.11.0
> >>> +1 for this RFC. I also once encounter such problem, and I use the same
> >> way to solve it. The addr parameter of mmap() syscall is only a hint instead
> of
> >> a must even the VMA is not occupied yet.
> >>> Thanks,
> >>> Jianfeng
> >> How do you know the VMA is not occupied?
> > I did by check /proc/self/maps.
> >
> >> I think the risk here is that the dynamic linker loaded some shared
> >> library in that VMA, and forcing MAP_FIXED is not a safe solution.
> >> What I have observed is that Linux will return a different VMA than the
> >> one hinted when there is already a mapping in the requested/hinted
> VMA.
> > IMO, that's not the target of this RFC. The target is to solve the situation (in
> current primary/secondary model) that kernel will not use the addr even
> there's no VMA on that addr. This is my understanding, Stephen, please
> correct me if I'm wrong.
> 
> The point I was trying to make is, that you do not know if there is a
> mapping or not in that address, and by using MAP_FIXED it will unmap
> whatever was in there before.

Oh, I missed that if there's conflict, the existing VMA will be unmapped. That's a bad effect.

> 
> So unless you parse /proc/self/maps and check that the VMA range is not
> being used, forcing MAP_FIXED is not safe.
> 
> >> I reckon this is a similar issue as we have with the multi-process model
> >> when we do not get the VMA requested for the huge-pages.
> >> AFAIK we do not have a robust solution for this issue other than restart
> >> the program and hope the dynamic linker does not map anything in the
> VMA
> >> ranges that we need to map from the primary. This is also assuming that
> >> the application does not allocate memory and maps things before calling
> >> eal_init as it could potentially use VMA ranges that we need in the
> >> secondary process.
> > This is another problem.
> 
> It is the same problem, VMA ranges that we need to map being already used.

Still two problems from my side:
(1) A VMA already exists on that addr/len range; conflict happens.
(2) Kernel will not allocate the VMA to DPDK even there is no VMA on that ranges; there's no conflict.

Thanks,
Jianfeng

> 
> Thanks,
> Sergio
> 
> >> The proposal for new secondary process model would solve these issues:
> >> http://dpdk.org/ml/archives/dev/2017-May/066147.html
> > And yes, this might happen to solve the targeted issue in this RFC. But
> before the new model is out, this patch seems a workable way for the
> original issue.
> >
> > Thanks,
> > Jianfeng
> >
> >> Thanks,
> >> Sergio
>