From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f41.google.com (mail-pa0-f41.google.com [209.85.220.41]) by dpdk.org (Postfix) with ESMTP id E3D0FDE6 for ; Fri, 3 Jul 2015 10:51:49 +0200 (CEST) Received: by paceq1 with SMTP id eq1so54242928pac.3 for ; Fri, 03 Jul 2015 01:51:49 -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=awWWzN3hCFqtUaSHUtKHvTWAvNoBpLDVo8fj4FzbKrQ=; b=ShAMLsGt3ZFwQgZ9F4edNMnvFLs3WLcjmelOwK9zS6sVYpIcVEhvOz+UNuWNI13u9I z7RmyvbGhgIqqqzfSbrFrGm/Qv9mZPxkX+NSC/ykWsrXahmqmznNIRkQ/n5zlP5i92qF XRM6qYnVO2euUs6RtNdlKSbpiqeaJxseSU22lmbcSrlLCjIT/dddrda6cvdWuoBsx04a +4QrPB+j0u4S9afZQLKvdGPjiIO9C351OcioRWCG2KR1WjBSCpIDkMI948g/XEKQpseu zA4qcS6pxjhmYxRBbjI1c8HmAmcKH4v6vHNDz7ttikTkoQlJxl2Si5/tXY3oCgod31du xlPw== X-Gm-Message-State: ALoCoQmh7/5QrNEFzymae8x8UensYKkSE6nbK3nwh8W+yfAnYQoZf4bg+IlM528EKPFMtc3IKwPR X-Received: by 10.70.15.1 with SMTP id t1mr74225110pdc.155.1435913509349; Fri, 03 Jul 2015 01:51:49 -0700 (PDT) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id ci12sm8261196pdb.41.2015.07.03.01.51.47 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Jul 2015 01:51:48 -0700 (PDT) Message-ID: <55964D24.1010106@igel.co.jp> Date: Fri, 03 Jul 2015 17:51:48 +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: Bruce Richardson References: <1435306705-11645-4-git-send-email-mukawa@igel.co.jp> <1435652668-3380-1-git-send-email-mukawa@igel.co.jp> <1435652668-3380-6-git-send-email-mukawa@igel.co.jp> <20150702102016.GB3828@bricha3-MOBL3> In-Reply-To: <20150702102016.GB3828@bricha3-MOBL3> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v7 05/12] eal: Fix uio mapping differences between linuxapp and bsdapp 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, 03 Jul 2015 08:51:50 -0000 On 2015/07/02 19:20, Bruce Richardson wrote: > On Tue, Jun 30, 2015 at 05:24:21PM +0900, Tetsuya Mukawa wrote: >> From: "Tetsuya.Mukawa" >> >> This patch fixes below. >> - bsdapp >> - Use map_id in pci_uio_map_resource(). >> - Fix interface of pci_map_resource(). >> - Move path variable of mapped_pci_resource structure to pci_map. >> - linuxapp >> - Remove redundant error message of linuxapp. >> >> 'pci_uio_map_resource()' is implemented in both linuxapp and bsdapp, >> but interface is different. The patch fixes the function of bsdapp >> to do same as linuxapp. After applying it, file descriptor should be >> opened and closed out of pci_map_resource(). >> >> Signed-off-by: Tetsuya Mukawa >> --- >> lib/librte_eal/bsdapp/eal/eal_pci.c | 118 ++++++++++++++++++------------ >> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 21 +++--- >> 2 files changed, 80 insertions(+), 59 deletions(-) >> > >> free_uio_res: >> + for (i = 0; i < map_idx; i++) >> + rte_free(maps[i].path); >> rte_free(uio_res); >> close_fd: >> close(dev->intr_handle.fd); > Comments on previous patch about merging error labels would also apply here. Right, I will fix it also. >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> index c3b259b..19620fe 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> @@ -116,17 +116,11 @@ pci_uio_map_secondary(struct rte_pci_device *dev) >> fd, (off_t)uio_res->maps[i].offset, >> (size_t)uio_res->maps[i].size, 0); >> if (mapaddr != uio_res->maps[i].addr) { >> - if (mapaddr == MAP_FAILED) >> - RTE_LOG(ERR, EAL, >> - "Cannot mmap device resource file %s: %s\n", >> - uio_res->maps[i].path, >> - strerror(errno)); >> - else >> - RTE_LOG(ERR, EAL, >> - "Cannot mmap device resource file %s to address: %p\n", >> - uio_res->maps[i].path, >> - uio_res->maps[i].addr); >> - >> + RTE_LOG(ERR, EAL, >> + "Cannot mmap device resource " >> + "file %s to address: %p\n", >> + uio_res->maps[i].path, >> + uio_res->maps[i].addr); >> close(fd); >> return -1; >> } >> @@ -353,8 +347,11 @@ pci_uio_map_resource(struct rte_pci_device *dev) >> >> /* allocate memory to keep path */ >> maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0); >> - if (maps[map_idx].path == NULL) >> + if (maps[map_idx].path == NULL) { >> + RTE_LOG(ERR, EAL, "Cannot allocate memory for " >> + "path: %s\n", strerror(errno)); > It's recommended not to split literal strings across multiple lines. This is > so that it's easy to find error messages in the source code. In this case, because > of the split, someone using git grep to search for the error message > "Cannot allocate memory for path" > would not be able to find it in the code. Longer lines are allowed in code for > literal strings. > > /Bruce > Sure, I will fix it. Tetsuya