diff mbox series

[v2,1/5] lib/stack: fix inconsistent weak / strong cas

Message ID 20200925174340.10014-2-steven.lariau@arm.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series lib/stack: improve lockfree C11 implementation |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Steven Lariau Sept. 25, 2020, 5:43 p.m. UTC 
  Fix cmpexchange usage of weak / strong.
The generated code is the same on x86 and ARM (there is no weak
cmpexchange), but the old usage was inconsistent.
For push and pop update size, weak is used because cmpexchange is inside
a loop.
For pop update root, strong is used even though cmpexchange is inside a
loop, because there may be a lot of operations to do in a loop iteration
(locate the new head).

Signed-off-by: Steven Lariau <steven.lariau@arm.com>
Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Acked-by: Gage Eads <gage.eads@intel.com>
---
 lib/librte_stack/rte_stack_lf_c11.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

David Marchand Sept. 28, 2020, 10:22 a.m. UTC | #1 
On Fri, Sep 25, 2020 at 7:44 PM Steven Lariau <steven.lariau@arm.com> wrote:
>
> Fix cmpexchange usage of weak / strong.
> The generated code is the same on x86 and ARM (there is no weak
> cmpexchange), but the old usage was inconsistent.
> For push and pop update size, weak is used because cmpexchange is inside
> a loop.
> For pop update root, strong is used even though cmpexchange is inside a
> loop, because there may be a lot of operations to do in a loop iteration
> (locate the new head).

Is this patch backport material?

Thanks.
Eads, Gage Sept. 28, 2020, 3:58 p.m. UTC | #2 
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, September 28, 2020 5:23 AM
> To: Steven Lariau <steven.lariau@arm.com>; Eads, Gage <gage.eads@intel.com>
> Cc: Olivier Matz <olivier.matz@6wind.com>; dev <dev@dpdk.org>; nd
> <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v2 1/5] lib/stack: fix inconsistent weak / strong
> cas
> 
> On Fri, Sep 25, 2020 at 7:44 PM Steven Lariau <steven.lariau@arm.com> wrote:
> >
> > Fix cmpexchange usage of weak / strong.
> > The generated code is the same on x86 and ARM (there is no weak
> > cmpexchange), but the old usage was inconsistent.
> > For push and pop update size, weak is used because cmpexchange is inside
> > a loop.
> > For pop update root, strong is used even though cmpexchange is inside a
> > loop, because there may be a lot of operations to do in a loop iteration
> > (locate the new head).
> 
> Is this patch backport material?

It's not a bugfix. It could help performance on a system with weak
cmpexchange -- e.g. the pop-update change would ensure no spurious
failures (which the code can handle, but would require another relatively
expensive pass through the pop loop.)

Thanks,
Gage
diff mbox series

Patch

diff --git a/lib/librte_stack/rte_stack_lf_c11.h b/lib/librte_stack/rte_stack_lf_c11.h
index 999359f08..1e0ea0bef 100644
--- a/lib/librte_stack/rte_stack_lf_c11.h
+++ b/lib/librte_stack/rte_stack_lf_c11.h
@@ -96,7 +96,7 @@  __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 		/* len is updated on failure */
 		if (__atomic_compare_exchange_n(&list->len,
 						&len, len - num,
-						0, __ATOMIC_ACQUIRE,
+						1, __ATOMIC_ACQUIRE,
 						__ATOMIC_ACQUIRE))
 			break;
 	}
@@ -149,7 +149,7 @@  __rte_stack_lf_pop_elems(struct rte_stack_lf_list *list,
 				(rte_int128_t *)&list->head,
 				(rte_int128_t *)&old_head,
 				(rte_int128_t *)&new_head,
-				1, __ATOMIC_RELEASE,
+				0, __ATOMIC_RELEASE,
 				__ATOMIC_RELAXED);
 	} while (success == 0);