From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f193.google.com (mail-wr0-f193.google.com [209.85.128.193]) by dpdk.org (Postfix) with ESMTP id 8E705968; Fri, 23 Jun 2017 19:08:52 +0200 (CEST) Received: by mail-wr0-f193.google.com with SMTP id z45so14013146wrb.2; Fri, 23 Jun 2017 10:08:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=9BKVLNFi9vxBpSMiywqyQLa97LtJPCPmIu3I4eGaQCM=; b=kyeEMYL8z6qbRuAV3ELovIcwPLTRf2bXCcUOwg+W2Fod3tE6bRuI0GeHSyGrQNjMjk IjC5KpGPLMbvnB1raHHb55hZkK9GTIqD9y+tBE71Dj/Uy9ntqOWprn/WkptadyqRnaae btKaUmNhaCyj8eY+aCIbvWIvIIj1swiGyqFx3t2dyQe9x2m/o/p4Xbdownf/hdSAiWwA ryFzn//pqfX3Fz0bSEUIdq0+TFxSgRSeleeXD0olh2B8iKaGsnagUPz9TS8hV4qTGLS/ XLyRd9WfrsGZxFajH/WzbCo1w43VCVRqsSyqlfIdr83oA7B2U11xj4KjWUPLYWrCvG0c 8YQw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=9BKVLNFi9vxBpSMiywqyQLa97LtJPCPmIu3I4eGaQCM=; b=VjYFenRqZ1Oyk319ffLNzsCMOLP3gHiQQgJezNGTmDQX7cb3DVvMxvlDrwJO3bulGm XSSdajiaohBR+3amQtcr4mp2RYSHdJICzo9mhDaB5Y5/h4bur6IOczbufMoiQfSrq5bj Tdq2VhHSi9iXiiheSZdBFME7SgYn9EW6JBRyY4eqFfyBBVXRprJ/Lr8yz0isBOJ7Pglk C0w1ddfokV2KIpopsydtZNPC0Zi9UCDPqP3F5Bt1SQRQ8qll9Sh2FrLalMLSSnKreYkM IccvcDum83o7bhN1eKkcurmfQ0+d2M2fBlA93qGfaUoWnPz9pMs1jvLnokoDyJQInW+i YXXw== X-Gm-Message-State: AKS2vOw0Aa+obsQDBdcK6Tu3O4r25cCfyH4SrW0VGMVg4Qr58Xefr3/B f/XryGYV7+P2tttdgKyYLYsOqppqlA== X-Received: by 10.223.143.77 with SMTP id p71mr6493588wrb.3.1498237732015; Fri, 23 Jun 2017 10:08:52 -0700 (PDT) MIME-Version: 1.0 Sender: jblunck@gmail.com Received: by 10.28.5.136 with HTTP; Fri, 23 Jun 2017 10:08:51 -0700 (PDT) In-Reply-To: <20170623101157.43dcafe3@platinum> References: <20170609102727.0eb7f39d@platinum> <20170609082937.21294-1-olivier.matz@6wind.com> <20170623101157.43dcafe3@platinum> From: Jan Blunck Date: Fri, 23 Jun 2017 19:08:51 +0200 X-Google-Sender-Auth: a9r3AaxnyLFrGE6r9Yk4TZ1lvMY Message-ID: To: Olivier Matz Cc: dev , Ilya Matveychikov , Adrien Mazarguil , sergio.gonzalez.monroy@intel.com, stable@dpdk.org Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH] eal: don't advertise a physical address when no hugepages X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Jun 2017 17:08:52 -0000 On Fri, Jun 23, 2017 at 10:11 AM, Olivier Matz wrote: > 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. > This approach looks better to me. Thanks, Jan