From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f171.google.com (mail-pd0-f171.google.com [209.85.192.171]) by dpdk.org (Postfix) with ESMTP id 3685EC79E for ; Fri, 26 Jun 2015 03:35:37 +0200 (CEST) Received: by pdjn11 with SMTP id n11so64456672pdj.0 for ; Thu, 25 Jun 2015 18:35:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=fxh+hi4LYGm6QrFh0GxfMveWeci2XgqakG/jcg5iKuU=; b=fd6bqb8D6Qwy+y92P4xpJGGBnk8sttvkuew284gH50hRuazjutNTjKKxGqSpg/VLw7 qZvjw17xWUOP6bgTlTokz/pEh0GtJRSTdRKHp09M3Xsw1fCyyMHP5IIK7JFZ4S7Ke15E 8pm+O38kRmoTD2joEsn15Hqgao8b4eGoV1LAuhiHCSOnGcN84q9CpOX3Lj85ADoiuQb5 +l5EJicJKQxoVr8vbDpRYreko4yFGlzWoJHsuP12GlwMng7xYAx84BYcPz5QHRnndmZ+ gpnwhND0hKniXtttmsfPnYxY1QREggv+0G7TSg5hbj0noo8KO+A4RocIOosn5/OaiEY/ WKvA== X-Gm-Message-State: ALoCoQnI3lNZAJKzibmDPtlcBLBwdghPRUz1DFwUCuiG9Ko2rbez0SuarjwNZOICnVB3qpDqENaP X-Received: by 10.70.137.66 with SMTP id qg2mr42766066pdb.77.1435282536610; Thu, 25 Jun 2015 18:35:36 -0700 (PDT) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id pr4sm31307049pbb.30.2015.06.25.18.35.35 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 25 Jun 2015 18:35:36 -0700 (PDT) Message-ID: <558CAC68.2000805@igel.co.jp> Date: Fri, 26 Jun 2015 10:35:36 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: David Marchand References: <1432014898-3543-2-git-send-email-mukawa@igel.co.jp> <1435202367-8887-1-git-send-email-mukawa@igel.co.jp> <1435202367-8887-4-git-send-email-mukawa@igel.co.jp> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v5 3/5] eal: Fix memory leaks and needless increment of pci_map_addr X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Jun 2015 01:35:37 -0000 On 2015/06/25 18:18, David Marchand wrote: > On Thu, Jun 25, 2015 at 5:19 AM, Tetsuya Mukawa > wrote: > > From: "Tetsuya.Mukawa" > > > This patch fixes following memory leaks. > - When open() is failed, uio_res and fds won't be freed in > pci_uio_map_resource(). > - When pci_map_resource() is failed but path is allocated correctly, > path and fds won't be freed in pci_uio_map_recource(). > - When pci_uio_unmap() is called, path should be freed. > > Also, fixes below. > - When pci_map_resource() is failed, mapaddr will be MAP_FAILED. > In this case, pci_map_addr should not be incremented in > pci_uio_map_resource(). > - To shrink code, move close(). > - Remove fail variable. > > Signed-off-by: Tetsuya Mukawa > > --- > lib/librte_eal/bsdapp/eal/eal_pci.c | 14 +++++++-- > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 51 > ++++++++++++++++++++----------- > 2 files changed, 44 insertions(+), 21 deletions(-) > > diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c > b/lib/librte_eal/bsdapp/eal/eal_pci.c > index 8e24fd1..b071f07 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_pci.c > +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c > @@ -235,7 +235,7 @@ pci_uio_map_resource(struct rte_pci_device *dev) > if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), > 0)) == NULL) { > RTE_LOG(ERR, EAL, > "%s(): cannot store uio mmap details\n", > __func__); > - return -1; > + goto close_fd; > } > > snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname); > @@ -262,8 +262,7 @@ pci_uio_map_resource(struct rte_pci_device *dev) > (mapaddr = pci_map_resource(NULL, devname, > (off_t)offset, > (size_t)maps[j].size) > ) == NULL) { > - rte_free(uio_res); > - return -1; > + goto free_uio_res; > } > > maps[j].addr = mapaddr; > @@ -274,6 +273,15 @@ pci_uio_map_resource(struct rte_pci_device *dev) > TAILQ_INSERT_TAIL(uio_res_list, uio_res, next); > > return 0; > + > +free_uio_res: > + rte_free(uio_res); > +close_fd: > + close(dev->intr_handle.fd); > + dev->intr_handle.fd = -1; > + dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; > + > + return -1; > } > > > Thinking about it, when something fails, don't you need to unmap pci > resources in uio_res->maps before freeing ? Yes, you are right. I will add it to linux code. So far BSD doesn't have unmap function. It will be added through other patch series I've already submitted. Regards, Tetsuya > > > -- > David Marchand