Re: [dpdk-stable] [PATCH] mem: fix allocation failure on non-NUMA kernel

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

On 05-Aug-20 4:21 PM, Nick Connolly wrote:
> 
> 
> On 05/08/2020 16:13, Nicolas Dichtel wrote:
>> Le 05/08/2020 à 16:53, Nick Connolly a écrit :
>> [snip]
>>>>>>> +    if (check_numa()) {
>>>>>>> +        ret = get_mempolicy(&cur_socket_id, NULL, 0, addr,
>>>>>>> +                    MPOL_F_NODE | MPOL_F_ADDR);
>>>>>>> +        if (ret < 0) {
>>>>>>> +            RTE_LOG(DEBUG, EAL, "%s(): get_mempolicy: %s\n",
>>>>>>> +                __func__, strerror(errno));
>>>>>>> +            goto mapped;
>>>>>>> +        } else if (cur_socket_id != socket_id) {
>>>>>>> +            RTE_LOG(DEBUG, EAL,
>>>>>>> +                    "%s(): allocation happened on wrong socket 
>>>>>>> (wanted %d,
>>>>>>> got %d)\n",
>>>>>>> +                __func__, socket_id, cur_socket_id);
>>>>>>> +            goto mapped;
>>>>>>> +        }
>>>>>>> +    } else {
>>>>>>> +        if (rte_socket_count() > 1)
>>>>>>> +            RTE_LOG(DEBUG, EAL, "%s(): not checking socket for 
>>>>>>> allocation
>>>>>>> (wanted %d)\n",
>>>>>>> +                    __func__, socket_id);
>>>>>> nit: maybe an higher log level like WARNING?
>>>>> Open to guidance here - my concern was that this is going to be 
>>>>> generated for
>>>>> every call to alloc_seg() and I'm not sure what the frequency will 
>>>>> be - I'm
>>>>> cautious about flooding the log with warnings under 'normal 
>>>>> running'.  Are the
>>>>> implications of running on a multi socket system with NUMA support 
>>>>> disabled in
>>>>> the kernel purely performance related for the DPDK or is there a 
>>>>> functional
>>>>> correctness issue as well?
>>>> Is it really a 'normal running' to have 
>>>> CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES in
>>>> dpdk and not CONFIG_NUMA in the kernel?
>>> I'm not an expert of DPDK, but I think it needs to be treated as 'normal
>>> running', for the following reasons:
>>>
>>> 1. The existing code in eal_memalloc_alloc_seg_bulk() is designed to
>>>     work even if check_numa() indicates that NUMA support is not 
>>> enabled:
>>>
>>>     #ifdef RTE_EAL_NUMA_AWARE_HUGEPAGES
>>>     if (check_numa()) {
>>>              oldmask = numa_allocate_nodemask();
>>>              prepare_numa(&oldpolicy, oldmask, socket);
>>>              have_numa = true;
>>>          }
>>>     #endif
>> The question was not to return an error, but to display a warning. So 
>> the code
>> will work (after your patch), no problem.
>>
>>> 2. The DPDK application could be built with
>>>     CONFIG_RTE_EAL_NUMA_AWARE_HUGE_PAGES and then the binary run on
>>>     different systems with and without NUMA support.
>> In a production environment, it seems odd to have a custom kernel and 
>> a generic
>> dpdk app, it's why I propose the log level WARNING (or NOTICE maybe?).
>> I let other comment about this, I don't have a strong opinion.
> Thanks - appreciate the input - I also have no strong opinions here and 
> am happy to be guided.

If there is a socket mismatch, wouldn't allocation fail anyway, which 
would result in an error message? Usually, when things fail, the first 
obvious thing to do is turn up the debug log. I'm inclined to leave it 
as DEBUG.

> 
> Regards,
> Nick


-- 
Thanks,
Anatoly