From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 3EF1369D4 for ; Thu, 1 Nov 2018 12:12:21 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Nov 2018 04:12:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,451,1534834800"; d="scan'208";a="246164894" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.72]) ([10.237.220.72]) by orsmga004.jf.intel.com with ESMTP; 01 Nov 2018 04:12:19 -0700 To: Alejandro Lucero Cc: dev References: <20181031172931.11894-1-alejandro.lucero@netronome.com> <20181031172931.11894-6-alejandro.lucero@netronome.com> From: "Burakov, Anatoly" Message-ID: <3d4ebdde-326a-811d-4bf7-52498495bade@intel.com> Date: Thu, 1 Nov 2018 11:12:17 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 5/7] mem: modify error message for DMA mask check 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: Thu, 01 Nov 2018 11:12:21 -0000 On 01-Nov-18 11:03 AM, Alejandro Lucero wrote: > > > On Thu, Nov 1, 2018 at 10:29 AM Burakov, Anatoly > > wrote: > > On 31-Oct-18 5:29 PM, Alejandro Lucero wrote: > > If DMA mask checks shows mapped memory out of the supported range > > specified by the DMA mask, nothing can be done but return an error > > an report the error. This can imply the app not being executed at > > all or precluding dynamic memory allocation once the app is running. > > In any case, we can advice the user to force IOVA as PA if currently > > IOVA being VA and user being root. > > > > Signed-off-by: Alejandro Lucero > > > --- > > General comment - legacy memory will also need this check, correct? > > > Yes, there is another patch adding this for both, legacy-mem and no-huge > options. > > >   lib/librte_eal/common/malloc_heap.c | 35 > +++++++++++++++++++++++++---- > >   1 file changed, 31 insertions(+), 4 deletions(-) > > > > diff --git a/lib/librte_eal/common/malloc_heap.c > b/lib/librte_eal/common/malloc_heap.c > > index 7d423089d..711622f19 100644 > > --- a/lib/librte_eal/common/malloc_heap.c > > +++ b/lib/librte_eal/common/malloc_heap.c > > @@ -5,8 +5,10 @@ > >   #include > >   #include > >   #include > > +#include > >   #include > >   #include > > +#include > >   #include > > > >   #include > > @@ -294,7 +296,6 @@ alloc_pages_on_heap(struct malloc_heap *heap, > uint64_t pg_sz, size_t elt_size, > >       size_t alloc_sz; > >       int allocd_pages; > >       void *ret, *map_addr; > > -     uint64_t mask; > > > >       alloc_sz = (size_t)pg_sz * n_segs; > > > > @@ -322,11 +323,37 @@ alloc_pages_on_heap(struct malloc_heap > *heap, uint64_t pg_sz, size_t elt_size, > >               goto fail; > >       } > > > > +     /* Once we have all the memseg lists configured, if there > is a dma mask > > +      * set, check iova addresses are not out of range. > Otherwise the device > > +      * setting the dma mask could have problems with the mapped > memory. > > +      * > > +      * There are two situations when this can happen: > > +      *      1) memory initialization > > +      *      2) dynamic memory allocation > > +      * > > +      * For 1), an error when checking dma mask implies app can > not be > > +      * executed. For 2) implies the new memory can not be added. > > +      */ > >       if (mcfg->dma_maskbits) { > >               if (rte_mem_check_dma_mask(mcfg->dma_maskbits)) { > > -                     RTE_LOG(ERR, EAL, > > -                             "%s(): couldn't allocate memory due > to DMA mask\n", > > -                             __func__); > > +                     /* Currently this can only happen if IOMMU > is enabled > > +                      * with RTE_ARCH_X86. It is not safe to use > this memory > > +                      * so returning an error here. > > I don't think it's RTE_ARCH_X86-only. It can be any other arch with an > IOMMU that's reporting addressing limitations. > > > Right, but it is just IOMMU hardware from this architecture having the > current limitation. > I was trying to just explain why this can happen but I can remove the > reference to specific >  architecture problems. > > > > +                      * > > +                      * If IOVA is VA, advice to try with > '--iova-mode pa' > > +                      * which could solve some situations when > IOVA VA is not > > +                      * really needed. > > +                      */ > > +                     uid_t user = getuid(); > > +                     if ((rte_eal_iova_mode() == RTE_IOVA_VA) && > user == 0) > > rte_eal_using_phys_addrs()? > > (the above function name is a bit of a misnomer, it really checks if > they are available, but not necessarily used - it will return true in > RTE_IOVA_VA mode if you're running as root) > > > rte_eal_iova_mode returns rte_eal_get_configuration()->iova_mode what >  is set during initialization. It can be PA not just because IOMMU (not > after the patch) > but because some PMD does not reports IOVA VA support or because UIO is > in use. > Checking for root is because IOVA PA can not be used if non root. You've misinterpreted my comment. rte_eal_using_phys_addrs() will effectively return true if you're running as root. There's no need for an uid check. The "misnomer" comment was about the rte_eal_using_phys_addrs() - it reads like it would return false in IOVA_VA mode, but in reality, it will return true even if IOVA_VA mode - it really should be named "rte_eal_phys_addrs_available()" rather than "rte_eal_using_phys_addrs()". This would make it clearer. > > > > +                             RTE_LOG(ERR, EAL, > > +                                     "%s(): couldn't allocate > memory due to DMA mask.\n" > > +                                     "Try with 'iova-mode=pa'\n", > > +                                     __func__); > > +                     else > > +                             RTE_LOG(ERR, EAL, > > +                                     "%s(): couldn't allocate > memory due to DMA mask\n", > > +                                     __func__); > > I don't think the error message is terribly descriptive. Looking at it > through the eyes of someone who sees it for the first time and who has > no idea what "iova-mode=pa" is, i think it would be more useful to word > it the following way: > > couldn't allocate memory due to IOVA exceeding limits of current DMA > mask. > [for non-using phys addrs case] Please try initializing EAL with > --iova-mode=pa parameter. > > > I'm happy with using your terrific description instead ;-) > Thanks! > > Also, generally newlines in RTE_LOG look ugly unless you indent the > line :) > > >                       goto fail; > >               } > >       } > > > > > -- > Thanks, > Anatoly > -- Thanks, Anatoly