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.
[...]
> --- 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.
