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

[Dmitry Kozlyuk]

On Tue, 9 Jun 2020 14:36:10 +0100
"Burakov, Anatoly" <anatoly.burakov at intel.com> wrote:

> On 03-Jun-20 12:03 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>
> 
> > +int
> > +eal_memseg_list_alloc(struct rte_memseg_list *msl, int reserve_flags)
> > +{
> > +	size_t page_sz, 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) {
> > +#ifndef RTE_EXEC_ENV_WINDOWS
> > +		/* The hint would be misleading on Windows, but this function
> > +		 * is called from many places, including common code,
> > +		 * so don't duplicate the message.
> > +		 */
> > +		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);
> > +		else
> > +			RTE_LOG(ERR, EAL, "Cannot reserve memory\n");
> > +#endif  
> 
> You're left without any error messages on Windows. How about:
> 
> const char *err_str = "Cannot reserve memory\n";
> #ifndef RTE_EXEC_ENV_WINDOWS
> if (rte_errno == EADDRNOTAVAIL)
>     err_str = ...
> #endif
> RTE_LOG(ERR, EAL, err_str);
> 
> or something like that?
> 

How about removing generic error message here completely and printing more
specific messages at call sites? In fact, almost all of them already do this.
It would be more helpful in tracking down errors.

-- 
Dmitry Kozlyuk