Re: [dpdk-dev] [PATCH 5/7] mem: modify error message for DMA mask check

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

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 <alejandro.lucero@netronome.com>
> ---

General comment - legacy memory will also need this check, correct?

>   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 <stddef.h>
>   #include <stdlib.h>
>   #include <stdio.h>
> +#include <unistd.h>
>   #include <stdarg.h>
>   #include <errno.h>
> +#include <sys/types.h>
>   #include <sys/queue.h>
>   
>   #include <rte_memory.h>
> @@ -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.

> +			 *
> +			 * 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_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.

Also, generally newlines in RTE_LOG look ugly unless you indent the line :)

>   			goto fail;
>   		}
>   	}
> 


-- 
Thanks,
Anatoly