Re: [ovs-dev] [PATCH V2] netdev-dpdk: fix memory leak

[Daniele Di Proietto]

I'm glad we can finally uncomment this code!

The patch looks good to me.

I made a few style changes and pushed this to master

Thanks

2016-08-04 2:49 GMT-07:00 Mark Kavanagh :

> DPDK v16.07 introduces the ability to free memzones.
> Up until this point, DPDK memory pools created in OVS could
> not be destroyed, thus incurring a memory leak.
>
> Leverage the DPDK v16.07 rte_mempool API to free DPDK
> mempools when their associated reference count reaches 0 (this
> indicates that the memory pool is no longer in use).
>
> Signed-off-by: Mark Kavanagh 
> ---
>
> v2->v1: rebase to head of master, and remove 'RFC' tag
>
>  lib/netdev-dpdk.c | 29 +++--
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index aaac0d1..ffcd35c 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -506,7 +506,7 @@ dpdk_mp_get(int socket_id, int mtu)
> OVS_REQUIRES(dpdk_mutex)
>  }
>
>  static void
> -dpdk_mp_put(struct dpdk_mp *dmp)
> +dpdk_mp_put(struct dpdk_mp *dmp) OVS_REQUIRES(dpdk_mutex)
>  {
>
>  if (!dmp) {
> @@ -514,15 +514,12 @@ dpdk_mp_put(struct dpdk_mp *dmp)
>  }
>
>  dmp->refcount--;
> -ovs_assert(dmp->refcount >= 0);
>
> -#if 0
> -/* I could not find any API to destroy mp. */
> -if (dmp->refcount == 0) {
> -list_delete(dmp->list_node);
> -/* destroy mp-pool. */
> -}
> -#endif
> +if (OVS_UNLIKELY(!dmp->refcount)) {
> +ovs_list_remove(>list_node);
> +rte_mempool_free(dmp->mp);
> + }
> +
>  }
>
>  static void
> @@ -928,16 +925,18 @@ netdev_dpdk_destruct(struct netdev *netdev)
>  {
>  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>
> +ovs_mutex_lock(_mutex);
>  ovs_mutex_lock(>mutex);
> +
>  rte_eth_dev_stop(dev->port_id);
>  free(ovsrcu_get_protected(struct ingress_policer *,
>>ingress_policer));
> -ovs_mutex_unlock(>mutex);
>
> -ovs_mutex_lock(_mutex);
>  rte_free(dev->tx_q);
>  ovs_list_remove(>list_node);
>  dpdk_mp_put(dev->dpdk_mp);
> +
> +ovs_mutex_unlock(>mutex);
>  ovs_mutex_unlock(_mutex);
>  }
>
> @@ -946,6 +945,9 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
>  {
>  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>
> +ovs_mutex_lock(_mutex);
> +ovs_mutex_lock(>mutex);
> +
>  /* Guest becomes an orphan if still attached. */
>  if (netdev_dpdk_get_vid(dev) >= 0) {
>  VLOG_ERR("Removing port '%s' while vhost device still attached.",
> @@ -961,15 +963,14 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
>  fatal_signal_remove_file_to_unlink(dev->vhost_id);
>  }
>
> -ovs_mutex_lock(>mutex);
>  free(ovsrcu_get_protected(struct ingress_policer *,
>>ingress_policer));
> -ovs_mutex_unlock(>mutex);
>
> -ovs_mutex_lock(_mutex);
>  rte_free(dev->tx_q);
>  ovs_list_remove(>list_node);
>  dpdk_mp_put(dev->dpdk_mp);
> +
> +ovs_mutex_unlock(>mutex);
>  ovs_mutex_unlock(_mutex);
>  }
>
> --
> 1.9.3
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH V2] netdev-dpdk: fix memory leak

[Mark Kavanagh]

DPDK v16.07 introduces the ability to free memzones.
Up until this point, DPDK memory pools created in OVS could
not be destroyed, thus incurring a memory leak.

Leverage the DPDK v16.07 rte_mempool API to free DPDK
mempools when their associated reference count reaches 0 (this
indicates that the memory pool is no longer in use).

Signed-off-by: Mark Kavanagh 
---

v2->v1: rebase to head of master, and remove 'RFC' tag

 lib/netdev-dpdk.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index aaac0d1..ffcd35c 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -506,7 +506,7 @@ dpdk_mp_get(int socket_id, int mtu) OVS_REQUIRES(dpdk_mutex)
 }
 
 static void
-dpdk_mp_put(struct dpdk_mp *dmp)
+dpdk_mp_put(struct dpdk_mp *dmp) OVS_REQUIRES(dpdk_mutex)
 {
 
 if (!dmp) {
@@ -514,15 +514,12 @@ dpdk_mp_put(struct dpdk_mp *dmp)
 }
 
 dmp->refcount--;
-ovs_assert(dmp->refcount >= 0);
 
-#if 0
-/* I could not find any API to destroy mp. */
-if (dmp->refcount == 0) {
-list_delete(dmp->list_node);
-/* destroy mp-pool. */
-}
-#endif
+if (OVS_UNLIKELY(!dmp->refcount)) {
+ovs_list_remove(>list_node);
+rte_mempool_free(dmp->mp);
+ }
+
 }
 
 static void
@@ -928,16 +925,18 @@ netdev_dpdk_destruct(struct netdev *netdev)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
 
+ovs_mutex_lock(_mutex);
 ovs_mutex_lock(>mutex);
+
 rte_eth_dev_stop(dev->port_id);
 free(ovsrcu_get_protected(struct ingress_policer *,
   >ingress_policer));
-ovs_mutex_unlock(>mutex);
 
-ovs_mutex_lock(_mutex);
 rte_free(dev->tx_q);
 ovs_list_remove(>list_node);
 dpdk_mp_put(dev->dpdk_mp);
+
+ovs_mutex_unlock(>mutex);
 ovs_mutex_unlock(_mutex);
 }
 
@@ -946,6 +945,9 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
 
+ovs_mutex_lock(_mutex);
+ovs_mutex_lock(>mutex);
+
 /* Guest becomes an orphan if still attached. */
 if (netdev_dpdk_get_vid(dev) >= 0) {
 VLOG_ERR("Removing port '%s' while vhost device still attached.",
@@ -961,15 +963,14 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev)
 fatal_signal_remove_file_to_unlink(dev->vhost_id);
 }
 
-ovs_mutex_lock(>mutex);
 free(ovsrcu_get_protected(struct ingress_policer *,
   >ingress_policer));
-ovs_mutex_unlock(>mutex);
 
-ovs_mutex_lock(_mutex);
 rte_free(dev->tx_q);
 ovs_list_remove(>list_node);
 dpdk_mp_put(dev->dpdk_mp);
+
+ovs_mutex_unlock(>mutex);
 ovs_mutex_unlock(_mutex);
 }
 
-- 
1.9.3

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev