Re: [PATCH v3 4/6] eal/memory: rename memory API to iova types

From: Thomas Monjalon <thomas@monjalon.net>
To: santosh <santosh.shukla@caviumnetworks.com>
Cc: dev@dpdk.org, olivier.matz@6wind.com,
	jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com,
	anatoly.burakov@intel.com
Subject: Re: [PATCH v3 4/6] eal/memory: rename memory API to iova types
Date: Fri, 03 Nov 2017 14:58:43 +0100	[thread overview]
Message-ID: <16080450.FyQkJaYVGX@xps> (raw)
In-Reply-To: <2572bad0-8023-3146-dfd1-1e0ef874fe70@caviumnetworks.com>

03/11/2017 12:35, santosh:
> On Friday 03 November 2017 04:41 PM, Thomas Monjalon wrote:
> > 20/10/2017 14:31, Santosh Shukla:
> >> Renamed memory translational api to _iova types.
> >> The following api renamed from:
> >>
> >> rte_mempool_populate_phys()
> >> rte_mempool_populate_phys_tab()
> > These functions still have "physical addresses" in their description.
> > It is not consistent.
> >
> > Please provide ABI compatibility for mempool functions.
> >
> >> rte_eal_using_phys_addrs()
> > Why renaming rte_eal_using_phys_addrs?
> > I think we need to review how it is used.
> > Maybe it requires a rework.
> >
> >> rte_mem_virt2phy()
> >> rte_dump_physmem_layout()
> >> rte_eal_get_physmem_layout()
> >> rte_eal_get_physmem_size()
> > Those 3 functions deal with physical memory layout.
> > I don't see a need to rename them.
> 
> In iova=va mode, those api will have va address.

Yes addresses can be virtual.
But it is still physically segmented.

> > But the dump function needs a change to avoid printing
> > "phys" even in VA case.
> >
> >> rte_malloc_virt2phy()
> >> rte_mem_phy2mch()
> > This last function was removed with Xen.
> > It is wrong to rename it in the release notes.
> > It should just be removed from the map file (I will send a patch).
> >
> >> To the following iova types api:
> >>
> >> rte_mempool_populate_iova()
> >> rte_mempool_populate_iova_tab()
> >> rte_eal_using_iova_addrs()
> >> rte_mem_virt2iova()
> > [...]
> >> rte_malloc_virt2iova()
> > I am not convinced by the names virt2iova.
> > I sounds like "virt to virt".
> > What about "virt2io" or "virt2io_addr"?
> 
> no, iova can be _pa or _va, its an io_addr indeed but
> I prefer virt2iova, same mentioned in deprecation notice (no strong opinion),
> 
> More suggestion on naming pl.?

I like virt2io_addr but virt2iova is not so bad.
I agree with Santosh that we need more opinions.

> > As the ABI is broken in EAL 17.11, we do not care about compatibility.
> > But we must keep an alias to the old function name in order to allow
> > a smooth API transition for applications.
> > I suggest to add static inline functions with the old names and set
> > the deprecated attribute.
> 
> ok, Will address in 18.02.

I would prefer these changes made in 17.11 as announced.
As you are not willing to work on it, I am trying to send some
updated patches.