[dpdk-dev] [PATCH] eal/service: fix exit by resetting service lcores
Aaron Conole
aconole at redhat.com
Tue Mar 10 20:14:27 CET 2020
Aaron Conole <aconole at redhat.com> writes:
> "Van Haaren, Harry" <harry.van.haaren at intel.com> writes:
>
>>> -----Original Message-----
>>> From: David Marchand <david.marchand at redhat.com>
>>> Sent: Tuesday, March 10, 2020 4:31 PM
>>> To: Van Haaren, Harry <harry.van.haaren at intel.com>
>>> Cc: dev <dev at dpdk.org>; Aaron Conole <aconole at redhat.com>
>>> Subject: Re: [PATCH] eal/service: fix exit by resetting service lcores
>>>
>>> On Tue, Mar 10, 2020 at 2:32 PM Harry van Haaren
>>> <harry.van.haaren at intel.com> wrote:
>>> >
>>> > This commit releases all service cores from thier role,
>>> > returning them to ROLE_RTE on rte_service_finalize().
>>> >
>>> > This may fix an issue relating to the service cores causing
>>> > a race-condition on eal_cleanup(), where the service core
>>> > could still be executing while the main thread has already
>>> > free-d the service memory, leading to a segfault.
>>>
>>> Adding rte_service_lcore_reset_all() just tells a (remaining) service
>>> lcore to quit its loop, but does not close the race on lcore_states.
>>>
>>> The backtrace shows the same.
>>>
>>> (gdb) bt full
>>> #0 rte_service_runner_func (arg=<optimized out>) at
>>> ../lib/librte_eal/common/rte_service.c:455
>>> service_mask = 1
>>> i = <optimized out>
>>> lcore = 1
>>> cs = 0x1003ea200
>>> #1 0x00007ffff72030ef in eal_thread_loop (arg=<optimized out>) at
>>> ../lib/librte_eal/linux/eal/eal_thread.c:153
>>> fct_arg = <optimized out>
>>> c = 0 '\000'
>>> n = <optimized out>
>>> ret = <optimized out>
>>> lcore_id = <optimized out>
>>> thread_id = 140737203603200
>>> m2s = 14
>>> s2m = 22
>>> cpuset = "1", '\000' <repeats 175 times>,
>>> "\200\000\000\000\000\000\000\000\221\354e\360\377\177", '\000'
>>> <repeats 65 times>
>>> __func__ = "eal_thread_loop"
>>> #2 0x00007ffff065ddd5 in start_thread () from /lib64/libpthread.so.0
>>> No symbol table info available.
>>> #3 0x00007ffff038702d in clone () from /lib64/libc.so.6
>>> No symbol table info available.
>>>
>>>
>>> I added a rte_eal_mp_wait_lcore(), to ensure that each service lcore
>>> _did_ quit its loop.
>>> @@ -123,6 +123,7 @@ rte_service_finalize(void)
>>> return;
>>>
>>> rte_service_lcore_reset_all();
>>> + rte_eal_mp_wait_lcore();
>>>
>>> rte_free(rte_services);
>>> rte_free(lcore_states);
>>>
>>>
>>> I can't reproduce with this.
>>
>> OK - that's good news, thanks for the quick testing & feedback.
>>
>> Agree with your analysis of the above, indeed waiting for the cores
>> explicitly seems the right solution to remove the race.
>>
>> Will I spin up a v2 patchset with your co-authored-by added and the above
>> change included?
>
> Please spin the v2 - I am currently testing with David's incremental on
> my setup now.
Additionally, with the incremental:
Acked-by: Aaron Conole <aconole at redhat.com>
Please make sure to cc stable at .
More information about the dev
mailing list