Re: [dpdk-dev] [PATCH v2 4/4] eventdev: relax smp barriers with c11 atomics
From: Thomas Monjalon <hidden>
Date: 2020-07-06 10:04:19
02/07/2020 07:26, Phil Yang:
The implementation-specific opaque data is shared between arm and cancel operations. The state flag acts as a guard variable to make sure the update of opaque data is synchronized. This patch uses c11 atomics with explicit one way memory barrier instead of full barriers rte_smp_w/rmb() to synchronize the opaque data between timer arm and cancel threads.
I think we should write C11 (uppercase). Please, in your explanations, try to be more specific. Naming fields may help to make things clear. [...]
quoted hunk ↗ jump to hunk
--- a/lib/librte_eventdev/rte_event_timer_adapter.h +++ b/lib/librte_eventdev/rte_event_timer_adapter.h@@ -467,7 +467,7 @@ struct rte_event_timer { * - op: RTE_EVENT_OP_NEW * - event_type: RTE_EVENT_TYPE_TIMER */ - volatile enum rte_event_timer_state state; + enum rte_event_timer_state state; /**< State of the event timer. */
Why do you remove the volatile keyword? It is not explained in the commit log. This change is triggering a warning in the ABI check: http://mails.dpdk.org/archives/test-report/2020-July/140440.html Moving from volatile to non-volatile is probably not an issue. I expect the code generated for the volatile case to work the same in non-volatile case. Do you confirm? In any case, we need an explanation and an ABI check exception.