Re: [dpdk-dev] [PATCH 3/3] app/testpmd: map anonymous memory for eth devices

[Shahaf Shuler <shahafs@mellanox.com>]

Monday, April 1, 2019 4:28 PM, Burakov, Anatoly:
> Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: map anonymous memory
> for eth devices
> 
> On 01-Apr-19 11:34 AM, Shahaf Shuler wrote:
> > Mempools can be populated with anonymous memory when using
> command
> > line parameter --mp-alloc=anon.
> >
> > Considering the mempools are going to be used by the net devices, it
> > is better to DMA map this memory.
> >
> > This patch add such mapping now that we have the APIs in place[1].
> >
> > [1] commit c33a675b6276 ("bus: introduce device level DMA memory
> > mapping")
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > ---
> >   app/test-pmd/testpmd.c | 62
> ++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 8c4ebc774c..6729469c6b 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -834,6 +834,61 @@ setup_extmem(uint32_t nb_mbufs, uint32_t
> mbuf_sz,
> > bool huge)
> >
> >   	return 0;
> >   }
> > +static void
> > +dma_unmap_cb(struct rte_mempool *mp __rte_unused, void *opaque
> __rte_unused,
> > +	     struct rte_mempool_memhdr *memhdr, unsigned mem_idx
> > +__rte_unused) {
> > +	uint16_t pid = 0;
> > +	int ret;
> > +
> > +	ret = rte_extmem_unregister(memhdr->addr, memhdr->len);
> > +	if (ret) {
> > +		TESTPMD_LOG(DEBUG,
> > +			    "unable to un-register addr 0x%p\n", memhdr-
> >addr);
> > +		return;
> > +	}
> > +	RTE_ETH_FOREACH_DEV(pid) {
> > +		struct rte_eth_dev *dev =
> > +			&rte_eth_devices[pid];
> > +
> > +		ret = rte_dev_dma_unmap(dev->device, memhdr->addr, 0,
> > +					memhdr->len);
> > +		if (ret) {
> > +			TESTPMD_LOG(DEBUG,
> > +				    "unable to DMA unmap addr 0x%p\n",
> > +				    memhdr->addr);
> > +		}
> > +	}
> 
> I would expect unmap *then* unregister.

Right, thanks. 

> 
> > +}
> > +
> > +static void
> > +dma_map_cb(struct rte_mempool *mp __rte_unused, void *opaque
> __rte_unused,
> > +	   struct rte_mempool_memhdr *memhdr, unsigned mem_idx
> __rte_unused)
> > +{
> > +	uint16_t pid = 0;
> > +	size_t page_size = sysconf(_SC_PAGESIZE);
> > +	int ret;
> > +
> > +	ret = rte_extmem_register(memhdr->addr, memhdr->len, NULL, 0,
> > +				  page_size);
> > +	if (ret) {
> > +		TESTPMD_LOG(DEBUG,
> > +			    "unable to register addr 0x%p\n", memhdr->addr);
> > +		return;
> > +	}
> > +	RTE_ETH_FOREACH_DEV(pid) {
> > +		struct rte_eth_dev *dev =
> > +			&rte_eth_devices[pid];
> > +
> > +		ret = rte_dev_dma_map(dev->device, memhdr->addr, 0,
> > +				      memhdr->len);
> > +		if (ret) {
> > +			TESTPMD_LOG(DEBUG,
> > +				    "unable to DMA map addr 0x%p\n",
> > +				    memhdr->addr);
> 
> Maybe mention the device/driver name as well? Otherwise it's hard to see
> which particular DMA map/unmap has failed, should one choose to debug-
> by-logs.

Will add. 

> 
> > +		}
> > +	}
> > +}
> >
> >   /*
> >    * Configuration initialisation done once at init time.
> > @@ -879,6 +934,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size,
> unsigned nb_mbuf,
> >   			}
> >   			rte_pktmbuf_pool_init(rte_mp, NULL);
> >   			rte_mempool_obj_iter(rte_mp, rte_pktmbuf_init,
> NULL);
> > +			rte_mempool_mem_iter(rte_mp, dma_map_cb,
> NULL);
> >   			break;
> >   		}
> >   	case MP_ALLOC_XMEM:
> > @@ -2460,8 +2516,12 @@ pmd_test_exit(void)
> >   		}
> >   	}
> >   	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++)
> > -		if (mempools[i])
> > +		if (mempools[i]) {
> > +			if (mp_alloc_type == MP_ALLOC_ANON)
> > +				rte_mempool_mem_iter(mempools[i],
> dma_unmap_cb,
> > +						     NULL);
> >   			rte_mempool_free(mempools[i]);
> > +		}
> >
> >   	printf("\nBye...\n");
> >   }
> >
> 
> 
> --
> Thanks,
> Anatoly