From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier Matz Subject: Re: [PATCH] eal: don't advertise a physical address when no hugepages Date: Fri, 23 Jun 2017 10:11:57 +0200 Message-ID: <20170623101157.43dcafe3@platinum> References: <20170609102727.0eb7f39d@platinum> <20170609082937.21294-1-olivier.matz@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev , matvejchikov@gmail.com, Adrien Mazarguil , sergio.gonzalez.monroy@intel.com, stable@dpdk.org To: Jan Blunck Return-path: Received: from mail-wr0-f170.google.com (mail-wr0-f170.google.com [209.85.128.170]) by dpdk.org (Postfix) with ESMTP id 3CD9D2C00 for ; Fri, 23 Jun 2017 10:11:59 +0200 (CEST) Received: by mail-wr0-f170.google.com with SMTP id r103so54987468wrb.0 for ; Fri, 23 Jun 2017 01:11:59 -0700 (PDT) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Jan, On Sat, 10 Jun 2017 10:31:22 +0200, Jan Blunck wrote: > On Fri, Jun 9, 2017 at 10:29 AM, Olivier Matz wrote: > > When populating a mempool with a virtual memory area, the mempool > > library expects to be able to get the physical address of each page. > > > > When started with --no-huge, the physical addresses may not be available > > because the pages are not locked in memory. It sometimes returns > > RTE_BAD_PHYS_ADDR, which makes the mempool_populate() function to fail. > > > > This was working before the commit cdc242f260e7 ("eal/linux: support > > running as unprivileged user"), because rte_mem_virt2phy() was returning > > 0 instead of RTE_BAD_PHYS_ADDR, which was seen as a valid physical > > address. > > > > Since --no-huge is a debug function that breaks the support of physical > > drivers, always set physical addresses to RTE_BAD_PHYS_ADDR in memzones > > or in rte_mem_virt2phy(), and ensure that mempool won't complain in that > > case. > > > > Fixes: cdc242f260e7 ("eal/linux: support running as unprivileged user") > > > > CC: stable@dpdk.org > > Signed-off-by: Olivier Matz > > --- > > lib/librte_eal/common/eal_common_memzone.c | 5 ++++- > > lib/librte_eal/linuxapp/eal/eal_memory.c | 7 +++++++ > > lib/librte_mempool/rte_mempool.c | 2 +- > > 3 files changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c > > index 3026e36b8..c465c8fc2 100644 > > --- a/lib/librte_eal/common/eal_common_memzone.c > > +++ b/lib/librte_eal/common/eal_common_memzone.c > > @@ -251,7 +251,10 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len, > > > > mcfg->memzone_cnt++; > > snprintf(mz->name, sizeof(mz->name), "%s", name); > > - mz->phys_addr = rte_malloc_virt2phy(mz_addr); > > + if (rte_eal_has_hugepages()) > > + mz->phys_addr = rte_malloc_virt2phy(mz_addr); > > + else > > + mz->phys_addr = RTE_BAD_PHYS_ADDR; > > Since you set phys_addrs_available to false rte_malloc_virt2phy() > anyway returns RTE_BAD_PHYS_ADDR so I believe the conditional isn't > necessary here. > > Rest of the patch looks good to me. The variable phys_addrs_available only impacts rte_mem_virt2phy(). Here, for memzones allocation, rte_malloc_virt2phy() is used, and it gets its physical address by retrieving it from the memseg structure. With the full patch, "dump_memzone" displays something like: Zone 0: name:, phys:0xffffffffffffffff, len:0x30100, [...] ... If I strip the memzone part, it displays: Zone 0: name:, phys:0x7fe382c62640, len:0x30100, [...] ... So I think we should either keep the patch as is, or change the memseg and malloc part like this (it's maybe better): --- a/lib/librte_eal/common/rte_malloc.c +++ b/lib/librte_eal/common/rte_malloc.c @@ -254,5 +254,7 @@ rte_malloc_virt2phy(const void *addr) const struct malloc_elem *elem = malloc_elem_from_data(addr); if (elem == NULL) return 0; + if (elem->ms->phys_addr == RTE_BAD_PHYS_ADDR) + return RTE_BAD_PHYS_ADDR; return elem->ms->phys_addr + ((uintptr_t)addr - (uintptr_t)elem->ms->addr); } diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c index 1c99852..2a401ca 100644 --- a/lib/librte_eal/linuxapp/eal/eal_memory.c +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c @@ -973,7 +973,7 @@ rte_eal_hugepage_init(void) strerror(errno)); return -1; } - mcfg->memseg[0].phys_addr = (phys_addr_t)(uintptr_t)addr; + mcfg->memseg[0].phys_addr = RTE_BAD_PHYS_ADDR; mcfg->memseg[0].addr = addr; mcfg->memseg[0].hugepage_sz = RTE_PGSIZE_4K; mcfg->memseg[0].len = internal_config.memory; Let me know what you are ok with this and I'll send a v2. Thanks, Olivier