From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id E3F86558C for ; Fri, 22 Jul 2016 18:03:19 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP; 22 Jul 2016 09:02:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,405,1464678000"; d="scan'208";a="738321365" Received: from smonroyx-mobl.ger.corp.intel.com (HELO [10.237.220.43]) ([10.237.220.43]) by FMSMGA003.fm.intel.com with ESMTP; 22 Jul 2016 09:02:05 -0700 From: Sergio Gonzalez Monroy To: Thomas Monjalon Cc: bruce.richardson@intel.com, dev@dpdk.org, michalx.kobylinski@intel.com, david.marchand@6wind.com, Michal Jastrzebski References: <1469024689-1076-1-git-send-email-michalx.k.jastrzebski@intel.com> <1469198030-8664-1-git-send-email-michalx.k.jastrzebski@intel.com> <2746518.l5JpmnHLoW@xps13> Message-ID: <95d86f88-9898-b630-0c3b-e0c8a72fdc69@intel.com> Date: Fri, 22 Jul 2016 17:02:05 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <2746518.l5JpmnHLoW@xps13> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] eal: fix check number of bytes from read function 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, 22 Jul 2016 16:03:20 -0000 On 22/07/2016 16:24, Thomas Monjalon wrote: > 2016-07-22 16:33, Michal Jastrzebski: >> v2: >> -moved close(fd) just after read. >> -when read() from fd we expect 8 bytes, so PFN_MASK_SIZE macro >> was introduced instead sizeof(uint64_t). >> -removed errno print when read returns less than 8 bytes > Looks better. > Note: this changelog should be below --- to avoid appearing in > the commit. > >> In rte_mem_virt2phy: Value returned from a function and indicating the >> number of bytes was ignored. This could cause a wrong pfn (page frame >> number) mask read from pagemap file. >> When read returns less than the number of sizeof(uint64_t) bytes, >> function rte_mem_virt2phy returns error. > Better title to explain the issue: > mem: fix check of physical address retrieval > >> +#define PFN_MASK_SIZE 8 >> + >> #ifdef RTE_LIBRTE_XEN_DOM0 >> int rte_xen_dom0_supported(void) >> { >> @@ -158,7 +160,7 @@ rte_mem_lock_page(const void *virt) >> phys_addr_t >> rte_mem_virt2phy(const void *virtaddr) >> { >> - int fd; >> + int fd, retval; >> uint64_t page, physaddr; >> unsigned long virt_pfn; >> int page_size; >> @@ -209,10 +211,19 @@ rte_mem_virt2phy(const void *virtaddr) >> close(fd); >> return RTE_BAD_PHYS_ADDR; >> } >> - if (read(fd, &page, sizeof(uint64_t)) < 0) { >> + >> + retval = read(fd, &page, sizeof(uint64_t)); > We could use PFN_MASK_SIZE here > >> + close(fd); >> + if (retval < 0) { >> RTE_LOG(ERR, EAL, "%s(): cannot read /proc/self/pagemap: %s\n", >> __func__, strerror(errno)); >> - close(fd); >> + > useless whitespace > >> + return RTE_BAD_PHYS_ADDR; >> + } else if (retval != PFN_MASK_SIZE) { >> + RTE_LOG(ERR, EAL, "%s(): read %d bytes from /proc/self/pagemap " >> + "but expected %d:\n", >> + __func__, retval, PFN_MASK_SIZE); >> + > useless whitespace > >> return RTE_BAD_PHYS_ADDR; >> } >> >> @@ -222,7 +233,7 @@ rte_mem_virt2phy(const void *virtaddr) >> */ >> physaddr = ((page & 0x7fffffffffffffULL) * page_size) >> + ((unsigned long)virtaddr % page_size); >> - close(fd); >> + >> return physaddr; >> } > If you and Sergio agree, I can make the minor changes before applying. Go for it! Sergio