[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dpdk-dev
Subject:    Re: [dpdk-dev] [PATCH v5 04/11] eal/mem: extract common code for memseg list initialization
From:       "Burakov, Anatoly" <anatoly.burakov () intel ! com>
Date:       2020-05-29 8:49:08
Message-ID: 45bf549a-e974-b4e4-19d9-dba3393c574e () intel ! com
[Download RAW message or body]

On 28-May-20 3:41 PM, Dmitry Kozlyuk wrote:
> On Thu, 28 May 2020 12:46:49 +0100
> "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:
> 
>> 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@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?
> 
> Supported, yes; meaningful, not quite: for Windows, we start with address 0
> (let the OS choose) and using the option can hardly help. Probably Linux and
> FreeBSD EALs should print this hint. For Windows, we can leave the option,
> but not print misleading hint.
> 

We keep rte_errno when exiting this function, do we not? How about we 
just check it in the caller? It's a bit of a hack, but will avoid 
OS-specific #ifdef-ery in the common code. Not sure if it's worth it 
though, over just having an #idef :) Up to you!

-- 
Thanks,
Anatoly
[prev in list] [next in list] [prev in thread] [next in thread]