[dpdk-dev] [PATCH v5 04/11] eal/mem: extract common code for memseg list initialization

[Burakov, Anatoly]

On 25-May-20 1:37 AM, Dmitry Kozlyuk wrote:
> All supported OS create memory segment lists (MSL) and reserve VA space
> for them in a nearly identical way. Move common code into EAL private
> functions to reduce duplication.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk at gmail.com>
> ---

<snip>

> +eal_memseg_list_alloc(struct rte_memseg_list *msl, int reserve_flags)
> +{
> +	uint64_t page_sz;
> +	size_t mem_sz;
> +	void *addr;
> +
> +	page_sz = msl->page_sz;
> +	mem_sz = page_sz * msl->memseg_arr.len;
> +
> +	addr = eal_get_virtual_area(
> +		msl->base_va, &mem_sz, page_sz, 0, reserve_flags);
> +	if (addr == NULL) {
> +		if (rte_errno == EADDRNOTAVAIL)
> +			RTE_LOG(ERR, EAL, "Cannot reserve %llu bytes at [%p] - "
> +				"please use '--" OPT_BASE_VIRTADDR "' option\n",
> +				(unsigned long long)mem_sz, msl->base_va);

Do all OS's support this EAL option?

> +		else
> +			RTE_LOG(ERR, EAL, "Cannot reserve memory\n");
> +		return -1;
> +	}
> +	msl->base_va = addr;
> +	msl->len = mem_sz;
> +
> +	RTE_LOG(DEBUG, EAL, "VA reserved for memseg list at %p, size %zx\n",
> +			addr, mem_sz);
> +

<snip>

>   
> -#define MEMSEG_LIST_FMT "memseg-%" PRIu64 "k-%i-%i"
>   static int
> -alloc_memseg_list(struct rte_memseg_list *msl, uint64_t page_sz,
> +memseg_list_init(struct rte_memseg_list *msl, uint64_t page_sz,
>   		int n_segs, int socket_id, int type_msl_idx)
>   {
> -	char name[RTE_FBARRAY_NAME_LEN];
> -
> -	snprintf(name, sizeof(name), MEMSEG_LIST_FMT, page_sz >> 10, socket_id,
> -		 type_msl_idx);
> -	if (rte_fbarray_init(&msl->memseg_arr, name, n_segs,
> -			sizeof(struct rte_memseg))) {
> -		RTE_LOG(ERR, EAL, "Cannot allocate memseg list: %s\n",
> -			rte_strerror(rte_errno));
> -		return -1;
> -	}
> -
> -	msl->page_sz = page_sz;
> -	msl->socket_id = socket_id;
> -	msl->base_va = NULL;
> -	msl->heap = 1; /* mark it as a heap segment */
> -
> -	RTE_LOG(DEBUG, EAL, "Memseg list allocated: 0x%zxkB at socket %i\n",
> -			(size_t)page_sz >> 10, socket_id);
> -
> -	return 0;
> +	return eal_memseg_list_init(
> +		msl, page_sz, n_segs, socket_id, type_msl_idx, true);

Here and in similar places: I wonder if there's value of keeping 
memseg_list_init function instead of just calling eal_memseg_list_init() 
directly from where this is called?

-- 
Thanks,
Anatoly