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:tho...@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.zh...@intel.com>
Cc: Anatoly Burakov <anatoly.bura...@intel.com>
Cc: sta...@dpdk.org
Signed-off-by: Darek Stojaczyk <dariusz.stojac...@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
