From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuanhan Liu Subject: Re: [PATCH] vhost: clean up per-socket mutex Date: Sun, 2 Jul 2017 07:25:05 +0800 Message-ID: <20170701232505.GD11626@yliu-home> References: <20170612212904.8787-1-daniel.verkamp@intel.com> <20170614081953.iybnhlwqq6o35ybf@dhcp-192-218.str.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Daniel Verkamp , dev@dpdk.org, "Maxime Coquelin (mcoqueli)" To: Jens Freimann Return-path: Received: from mail-pf0-f196.google.com (mail-pf0-f196.google.com [209.85.192.196]) by dpdk.org (Postfix) with ESMTP id 1052B2B99 for ; Sun, 2 Jul 2017 01:25:12 +0200 (CEST) Received: by mail-pf0-f196.google.com with SMTP id c24so6792184pfe.1 for ; Sat, 01 Jul 2017 16:25:12 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20170614081953.iybnhlwqq6o35ybf@dhcp-192-218.str.redhat.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Jun 14, 2017 at 10:19:53AM +0200, Jens Freimann wrote: > On Mon, Jun 12, 2017 at 02:29:04PM -0700, Daniel Verkamp wrote: > > vsocket->conn_mutex was allocated with pthread_mutex_init() but never > > freed with pthread_mutex_destroy(). This is a potential memory leak, > > depending on how pthread_mutex_t is implemented. > > > > Signed-off-by: Daniel Verkamp > > --- > > lib/librte_vhost/socket.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c > > index c7f99b0..9720773 100644 > > --- a/lib/librte_vhost/socket.c > > +++ b/lib/librte_vhost/socket.c > > @@ -636,6 +636,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags) > > vsocket->reconnect = !(flags & RTE_VHOST_USER_NO_RECONNECT); > > if (vsocket->reconnect && reconn_tid == 0) { > > if (vhost_user_reconnect_init() < 0) { > > + pthread_mutex_destroy(&vsocket->conn_mutex); > > free(vsocket->path); > > free(vsocket); > > goto out; > > @@ -646,6 +647,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags) > > } > > ret = create_unix_socket(vsocket); > > if (ret < 0) { > > + pthread_mutex_destroy(&vsocket->conn_mutex); > > free(vsocket->path); > > free(vsocket); > > goto out; > > @@ -724,6 +726,7 @@ rte_vhost_driver_unregister(const char *path) > > } > > pthread_mutex_unlock(&vsocket->conn_mutex); > > > > + pthread_mutex_destroy(&vsocket->conn_mutex); > > Seems like we never do it, but shouldn't we check the return value > here? I think so. There are more: pthread_mutex_init/lock, etc. We probably should make another patch to fix it. For this patch, I'd like to apply as it is. So applied to dpdk-next-virtio. Thanks. --yliu