Re: [dpdk-dev] [PATCH] eal: initialize alarms early

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

On 28-Mar-19 10:46 AM, Thomas Monjalon wrote:
> 28/03/2019 11:43, Bruce Richardson:
>> On Wed, Mar 27, 2019 at 11:42:40PM +0100, Thomas Monjalon wrote:
>>> 27/03/2019 21:33, Stojaczyk, Dariusz:
>>>> From: Thomas Monjalon [mailto:thomas@monjalon.net]
>>>>> 26/03/2019 19:43, Darek Stojaczyk:
>>>>>> We currently initialize rte_alarms after starting
>>>>>> to listen for IPC hotplug requests, which gives
>>>>>> us a data race window. Upon receiving such hotplug
>>>>>> request we always try to set an alarm and this
>>>>>> obviously doesn't work if the alarms weren't
>>>>>> initialized yet.
>>>>>>
>>>>>> To fix it, we initialize alarms before starting
>>>>>> to listen for IPC hotplug messages. Specifically,
>>>>>> we move rte_eal_alarm_init() right after
>>>>>> rte_eal_intr_init() as it makes some sense to
>>>>>> keep those two close to each other.
>>>>>
>>>>> I wonder which regression it will bring :)
>>>>> The experience shows that we cannot touch this function
>>>>> without introducing a regression. Please check twice.
>>>>
>>>> Hah, ok - I'll check again the possible outcomes of this.
>>>>
>>>>>
>>>>>> Fixes: 244d5130719c ("eal: enable hotplug on multi-process")
>>>>>> Cc: Qi Zhang <qi.z.zhang@intel.com>
>>>>>> Cc: Anatoly Burakov <anatoly.burakov@intel.com>
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
>>>>>> ---
>>>>>>   lib/librte_eal/linux/eal/eal.c | 12 ++++++------
>>>>>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>>>>
>>>>> You probably need to update the FreeBSD version too.
>>>>>
>>>>
>>>> Oh, that I cannot do. First of all, in bsd code I don't see
>>>> rte_mp_dev_hotplug_init() called anywhere, as if bsd
>>>> did not listen for IPC hotplug messages at all and hence did
>>>> not have any data race in this area. Second, I would be
>>>> afraid to touch any bsd code as I'm not running any bsd
>>>> system.
>>>
>>> The problem is the consistency between OSes.
>>> May you ask help here? Bruce is maintaining the FreeBSD side.
>>>
>> I don't think we support IPC or hotplug on BSD, so I don't think this patch
>> is relevant on the BSD side.
> 
> Yes, but then, the initialization order is different on Linux and BSD.
> I'm thinking about consistency and maintenance ease.
> 

 From my cursory inspection, nothing should be broken on FreeBSD as a 
result of moving alarm API init earlier.

-- 
Thanks,
Anatoly