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