From 4ee8dc7f6650a3d56b616a15c7d298523d63278f Mon Sep 17 00:00:00 2001 From: Steven Lariau Date: Fri, 25 Sep 2020 18:43:39 +0100 Subject: [PATCH] stack: relax pop CAS ordering Replace the store-release by relaxed for the CAS success at the end of pop. Release isn't needed, because there is not write to data that need to be synchronized. The only preceding write is when the length is decreased, but the length CAS loop already ensures the right synchronization. The situation to avoid is when a thread sees the old length but the new list, that doesn't have enough items for pop to success. But the CAS success on length before the pop loop ensures any core reads and updates the latest length, preventing this situation. The store-release is also used to make sure that the items are read before the head is updated, in order to prevent a core in pop to read an incorrect value because another core rewrites it with push. But this isn't needed, because items are read only when removed from the used list. Right after this, they are pushed to the free list, and the store-release in push makes sure the items are read before they are visible in the free list. Signed-off-by: Steven Lariau Reviewed-by: Dharmik Thakkar Reviewed-by: Ruifeng Wang Acked-by: Gage Eads --- lib/librte_stack/rte_stack_lf_c11.h | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/librte_stack/rte_stack_lf_c11.h b/lib/librte_stack/rte_stack_lf_c11.h index adb9f590de..8403196d51 100644 --- a/lib/librte_stack/rte_stack_lf_c11.h +++ b/lib/librte_stack/rte_stack_lf_c11.h @@ -141,11 +141,25 @@ __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list, new_head.top = tmp; new_head.cnt = old_head.cnt + 1; + /* + * The CAS should have release semantics to ensure that + * items are read before the head is updated. + * But this function is internal, and items are read + * only when __rte_stack_lf_pop calls this function to + * pop items from used list. + * Then, those items are pushed to the free list. + * Push uses a CAS store-release on head, which makes + * sure that items are read before they are pushed to + * the free list, without need for a CAS release here. + * This CAS could also be used to ensure that the new + * length is visible before the head update, but + * acquire semantics on the length update is enough. + */ success = rte_atomic128_cmp_exchange( (rte_int128_t *)&list->head, (rte_int128_t *)&old_head, (rte_int128_t *)&new_head, - 0, __ATOMIC_RELEASE, + 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED); } while (success == 0); -- 2.20.1