[v3] kni: optimize alloc queue release

Message ID 20220413145717.3661947-1-s.v.naga.harish.k@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v3] kni: optimize alloc queue release |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

Naga Harish K, S V April 13, 2022, 2:57 p.m. UTC
  the kni alloc queue is filled with physical addresses of mbufs
for kernel consumption. Any unused mbufs in the alloc queue are
freed during shutdown sequence in rte_kni_release.

In the current existing implementation, for freeing one entry of
alloc queue all the objects of the mempool are traversed. This
process is repeated for all the objects of the alloc queue which
consumes lot of cpu cycles.

Instead of using mempool object iteration method,use
``rte_mem_iova2virt()`` api to get the virtual address
for the physical addresses of alloc_q objects.
This speeds up the freeing process.

Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
v2:
* fix checkpatch errors

v3:
* fix commit message as per review comments
---
 lib/kni/rte_kni.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)
  

Comments

Jayatheerthan, Jay April 14, 2022, 3:20 p.m. UTC | #1
> -----Original Message-----
> From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Sent: Wednesday, April 13, 2022 8:27 PM
> To: ferruh.yigit@xilinx.com; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; dev@dpdk.org
> Subject: [PATCH v3] kni: optimize alloc queue release
> 
> the kni alloc queue is filled with physical addresses of mbufs

nitpick: "The" instead of "the".

> for kernel consumption. Any unused mbufs in the alloc queue are
> freed during shutdown sequence in rte_kni_release.
> 
> In the current existing implementation, for freeing one entry of

nitpick: current and existing are redundant. Can keep one.

> alloc queue all the objects of the mempool are traversed. This
> process is repeated for all the objects of the alloc queue which
> consumes lot of cpu cycles.
> 
> Instead of using mempool object iteration method,use
> ``rte_mem_iova2virt()`` api to get the virtual address
> for the physical addresses of alloc_q objects.
> This speeds up the freeing process.
> 
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> ---
> v2:
> * fix checkpatch errors
> 
> v3:
> * fix commit message as per review comments
> ---
>  lib/kni/rte_kni.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/kni/rte_kni.c b/lib/kni/rte_kni.c
> index 7971c56bb4..f443e5b2fc 100644
> --- a/lib/kni/rte_kni.c
> +++ b/lib/kni/rte_kni.c
> @@ -375,26 +375,19 @@ va2pa_all(struct rte_mbuf *mbuf)
>  }
> 
>  static void
> -obj_free(struct rte_mempool *mp __rte_unused, void *opaque, void *obj,
> -		unsigned obj_idx __rte_unused)
> -{
> -	struct rte_mbuf *m = obj;
> -	void *mbuf_phys = opaque;
> -
> -	if (va2pa(m) == mbuf_phys)
> -		rte_pktmbuf_free(m);
> -}
> -
> -static void
> -kni_free_fifo_phy(struct rte_mempool *mp, struct rte_kni_fifo *fifo)
> +kni_free_fifo_phy(struct rte_kni_fifo *fifo)
>  {
>  	void *mbuf_phys;
>  	int ret;
> +	struct rte_mbuf *m;
> 
>  	do {
>  		ret = kni_fifo_get(fifo, &mbuf_phys, 1);
> -		if (ret)
> -			rte_mempool_obj_iter(mp, obj_free, mbuf_phys);
> +		if (ret) {
> +			m = (struct rte_mbuf *)
> +				rte_mem_iova2virt((rte_iova_t)mbuf_phys);
> +			rte_pktmbuf_free(m);
> +		}
>  	} while (ret);
>  }
> 
> @@ -440,7 +433,7 @@ rte_kni_release(struct rte_kni *kni)
>  	if (kni_fifo_count(kni->rx_q))
>  		RTE_LOG(ERR, KNI, "Fail to free all Rx-q items\n");
> 
> -	kni_free_fifo_phy(kni->pktmbuf_pool, kni->alloc_q);
> +	kni_free_fifo_phy(kni->alloc_q);
>  	kni_free_fifo(kni->tx_q);
>  	kni_free_fifo(kni->free_q);
> 
> --
> 2.23.0
  
Naga Harish K, S V April 14, 2022, 5:38 p.m. UTC | #2
Hi Jay,

> -----Original Message-----
> From: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Sent: Thursday, April 14, 2022 8:51 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>;
> ferruh.yigit@xilinx.com; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v3] kni: optimize alloc queue release
> 
> > -----Original Message-----
> > From: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Sent: Wednesday, April 13, 2022 8:27 PM
> > To: ferruh.yigit@xilinx.com; Yigit, Ferruh <ferruh.yigit@intel.com>
> > Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; dev@dpdk.org
> > Subject: [PATCH v3] kni: optimize alloc queue release
> >
> > the kni alloc queue is filled with physical addresses of mbufs
> 
> nitpick: "The" instead of "the".

 Updated in v4 patch.

> 
> > for kernel consumption. Any unused mbufs in the alloc queue are freed
> > during shutdown sequence in rte_kni_release.
> >
> > In the current existing implementation, for freeing one entry of
> 
> nitpick: current and existing are redundant. Can keep one.

Updated in v4 patch.

> 
> > alloc queue all the objects of the mempool are traversed. This process
> > is repeated for all the objects of the alloc queue which consumes lot
> > of cpu cycles.
> >
> > Instead of using mempool object iteration method,use
> > ``rte_mem_iova2virt()`` api to get the virtual address for the
> > physical addresses of alloc_q objects.
> > This speeds up the freeing process.
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > ---
> > v2:
> > * fix checkpatch errors
> >
> > v3:
> > * fix commit message as per review comments
> > ---
> >  lib/kni/rte_kni.c | 23 ++++++++---------------
> >  1 file changed, 8 insertions(+), 15 deletions(-)
> >
> > diff --git a/lib/kni/rte_kni.c b/lib/kni/rte_kni.c index
> > 7971c56bb4..f443e5b2fc 100644
> > --- a/lib/kni/rte_kni.c
> > +++ b/lib/kni/rte_kni.c
> > @@ -375,26 +375,19 @@ va2pa_all(struct rte_mbuf *mbuf)  }
> >
> >  static void
> > -obj_free(struct rte_mempool *mp __rte_unused, void *opaque, void
> *obj,
> > -		unsigned obj_idx __rte_unused)
> > -{
> > -	struct rte_mbuf *m = obj;
> > -	void *mbuf_phys = opaque;
> > -
> > -	if (va2pa(m) == mbuf_phys)
> > -		rte_pktmbuf_free(m);
> > -}
> > -
> > -static void
> > -kni_free_fifo_phy(struct rte_mempool *mp, struct rte_kni_fifo *fifo)
> > +kni_free_fifo_phy(struct rte_kni_fifo *fifo)
> >  {
> >  	void *mbuf_phys;
> >  	int ret;
> > +	struct rte_mbuf *m;
> >
> >  	do {
> >  		ret = kni_fifo_get(fifo, &mbuf_phys, 1);
> > -		if (ret)
> > -			rte_mempool_obj_iter(mp, obj_free, mbuf_phys);
> > +		if (ret) {
> > +			m = (struct rte_mbuf *)
> > +				rte_mem_iova2virt((rte_iova_t)mbuf_phys);
> > +			rte_pktmbuf_free(m);
> > +		}
> >  	} while (ret);
> >  }
> >
> > @@ -440,7 +433,7 @@ rte_kni_release(struct rte_kni *kni)
> >  	if (kni_fifo_count(kni->rx_q))
> >  		RTE_LOG(ERR, KNI, "Fail to free all Rx-q items\n");
> >
> > -	kni_free_fifo_phy(kni->pktmbuf_pool, kni->alloc_q);
> > +	kni_free_fifo_phy(kni->alloc_q);
> >  	kni_free_fifo(kni->tx_q);
> >  	kni_free_fifo(kni->free_q);
> >
> > --
> > 2.23.0
  

Patch

diff --git a/lib/kni/rte_kni.c b/lib/kni/rte_kni.c
index 7971c56bb4..f443e5b2fc 100644
--- a/lib/kni/rte_kni.c
+++ b/lib/kni/rte_kni.c
@@ -375,26 +375,19 @@  va2pa_all(struct rte_mbuf *mbuf)
 }
 
 static void
-obj_free(struct rte_mempool *mp __rte_unused, void *opaque, void *obj,
-		unsigned obj_idx __rte_unused)
-{
-	struct rte_mbuf *m = obj;
-	void *mbuf_phys = opaque;
-
-	if (va2pa(m) == mbuf_phys)
-		rte_pktmbuf_free(m);
-}
-
-static void
-kni_free_fifo_phy(struct rte_mempool *mp, struct rte_kni_fifo *fifo)
+kni_free_fifo_phy(struct rte_kni_fifo *fifo)
 {
 	void *mbuf_phys;
 	int ret;
+	struct rte_mbuf *m;
 
 	do {
 		ret = kni_fifo_get(fifo, &mbuf_phys, 1);
-		if (ret)
-			rte_mempool_obj_iter(mp, obj_free, mbuf_phys);
+		if (ret) {
+			m = (struct rte_mbuf *)
+				rte_mem_iova2virt((rte_iova_t)mbuf_phys);
+			rte_pktmbuf_free(m);
+		}
 	} while (ret);
 }
 
@@ -440,7 +433,7 @@  rte_kni_release(struct rte_kni *kni)
 	if (kni_fifo_count(kni->rx_q))
 		RTE_LOG(ERR, KNI, "Fail to free all Rx-q items\n");
 
-	kni_free_fifo_phy(kni->pktmbuf_pool, kni->alloc_q);
+	kni_free_fifo_phy(kni->alloc_q);
 	kni_free_fifo(kni->tx_q);
 	kni_free_fifo(kni->free_q);