| Message ID | 20210224212018.17576-1-honnappa.nagarahalli@arm.com |
|---|---|
| Headers | show |
| Series | Use correct memory ordering in eal functions | expand |
On Wed, 24 Feb 2021 15:20:13 -0600 Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> wrote: > rte_eal_remote_launch and rte_eal_wait_lcore need to provide > correct memory ordering to address the data communication from > main core to worker core and vice versa. > > There are 2 use cases: > 1) All the store operations (meant for worker) by main core > should be visible to worker core before the worker core runs the > assigned function > > 2) All the store operations by the worker core should be visible > to the main core after rte_eal_wait_lcore returns. > > For the data that needs to be communicated to the worker after > the rte_eal_remote_launch returns, load-acquire and store-release > semantics should be used (just like any other writer-reader use case). > > For the main to worker communication, the pointer to function > to execute is used as the guard variable. Hence, resetting of > the function pointer is important. > > For the worker to main communication, the existing code uses the > lcore state as the guard variable. However, it looks like > the FINISHED state is not really required. Hence the FINISHED state > is removed before using the state as the guard variable. I would like > some feedback on why the FINISHED state is required. I have not > paid attention to what it means for backward compatibility. > If it is decided to remove this state, documentation changes are > required. > Most likely all use of volatile in DPDK is suspect. Perhaps we should re-enable the "volatile considered harmful" warning in checkpatch
<snip> > > > rte_eal_remote_launch and rte_eal_wait_lcore need to provide correct > > memory ordering to address the data communication from main core to > > worker core and vice versa. > > > > There are 2 use cases: > > 1) All the store operations (meant for worker) by main core should be > > visible to worker core before the worker core runs the assigned > > function > > > > 2) All the store operations by the worker core should be visible to > > the main core after rte_eal_wait_lcore returns. > > > > For the data that needs to be communicated to the worker after the > > rte_eal_remote_launch returns, load-acquire and store-release > > semantics should be used (just like any other writer-reader use case). > > > > For the main to worker communication, the pointer to function to > > execute is used as the guard variable. Hence, resetting of the > > function pointer is important. > > > > For the worker to main communication, the existing code uses the lcore > > state as the guard variable. However, it looks like the FINISHED state > > is not really required. Hence the FINISHED state is removed before > > using the state as the guard variable. I would like some feedback on > > why the FINISHED state is required. I have not paid attention to what > > it means for backward compatibility. > > If it is decided to remove this state, documentation changes are > > required. > > > > > Most likely all use of volatile in DPDK is suspect. > Perhaps we should re-enable the "volatile considered harmful" warning in > checkpatch Agree.
On Mon, Oct 25, 2021 at 6:53 AM Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> wrote: > > v3: > a) Added Fixes, Cc:stable#dpdk.org in 1/6 > b) Merged 3/6 & 4/6 and moved after the first commit in the series > c) Merged 2/6 & 5/6 as they need to be in a single commit > d) Removed use of volatile in 6/6 (Konstantin) > > rte_eal_remote_launch and rte_eal_wait_lcore need to provide > correct memory ordering to address the data communication from > main core to worker core. > > There are 2 use cases: > 1) All the store operations (meant for worker) by main core > should be visible to worker core before the worker core runs the > assigned function > > 2) All the store operations by the worker core should be visible > to the main core after rte_eal_wait_lcore returns. > > For the data that needs to be communicated to the worker after > the rte_eal_remote_launch returns, load-acquire and store-release > semantics should be used. > > For the main to worker communication, the pointer to function > to execute is used as the guard variable. Hence, resetting of > the function pointer is important. > > For the worker to main communication, the existing code uses the > lcore state as the guard variable. However, it looks like > the FINISHED state is not really required. Hence the FINISHED state > is removed before using the state as the guard variable. > > Honnappa Nagarahalli (4): > eal: reset lcore function pointer and argument > eal: lcore state FINISHED is not required > eal: ensure correct memory ordering > test/ring: use relaxed barriers for ring stress test > > app/test/test_ring_stress_impl.h | 18 +++---- > drivers/event/dpaa2/dpaa2_eventdev_selftest.c | 2 +- > drivers/event/octeontx/ssovf_evdev_selftest.c | 2 +- > drivers/event/sw/sw_evdev_selftest.c | 4 +- > examples/l2fwd-keepalive/main.c | 3 +- > lib/eal/common/eal_common_launch.c | 13 ++--- > lib/eal/freebsd/eal_thread.c | 45 +++++++++++++---- > lib/eal/include/rte_launch.h | 21 ++++---- > lib/eal/include/rte_service.h | 4 +- > lib/eal/linux/eal_thread.c | 48 +++++++++++++------ > lib/eal/windows/eal_thread.c | 48 +++++++++++++------ > 11 files changed, 132 insertions(+), 76 deletions(-) Tweaked commit titles, removed deprecation notice and updated release notes in patch 2. Series applied, thanks. -- David Marchand