19/12/2018 17:09, Erik Gabriel Carrillo:
> rte_timer_manage() adds expired timers to a "run list", and walks the
> list, transitioning each timer from the PENDING to the RUNNING state.
> If another lcore resets or stops the timer at precisely this
> moment, the timer state would instead be set to CONFIG by that other
> lcore, which would cause timer_manage() to skip over it. This is
> expected behavior.
>
> However, if a timer expires quickly enough, there exists the
> following race condition that causes the timer_manage() routine to
> misinterpret a timer in CONFIG state, resulting in lost timers:
>
> - Thread A:
> - starts a timer with rte_timer_reset()
> - the timer is moved to CONFIG state
> - the spinlock associated with the appropriate skiplist is acquired
> - timer is inserted into the skiplist
> - the spinlock is released
> - Thread B:
> - executes rte_timer_manage()
> - find above timer as expired, add it to run list
> - walk run list, see above timer still in CONFIG state, unlink it from
> run list and continue on
> - Thread A:
> - move timer to PENDING state
> - return from rte_timer_reset()
> - timer is now in PENDING state, but not actually linked into a
> pending list or a run list and will never get processed further
> by rte_timer_manage()
>
> This commit fixes this race condition by only releasing the spinlock
> after the timer state has been transitioned from CONFIG to PENDING,
> which prevents rte_timer_manage() from seeing an incorrect state.
>
> Fixes: 9b15ba895b9f ("timer: use a skip list")
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carri...@intel.com>
> Reviewed-by: Gavin Hu <gavin...@arm.com>
+Cc: sta...@dpdk.org
Applied, thanks
