Re: [dpdk-dev] [PATCH v3 4/6] eal/memory: rename memory API to iovatypes

["Jonas Pfefferle1" <JPF@zurich.ibm.com>]

"dev" <dev-bounces@dpdk.org> wrote on 11/03/2017 02:58:43 PM:

> 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
> Date: 11/03/2017 02:59 PM
> Subject: Re: [dpdk-dev] [PATCH v3 4/6] eal/memory: rename memory API
> to iova types
> Sent by: "dev" <dev-bounces@dpdk.org>
>
> 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.

virt2iova +1

Another suggestion va2iova to be consistent with the names

>
> > > 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.
>