kohler / click

The Click modular router: fast modular packet processing and analysis
Other
735 stars 322 forks source link

Spinlock does not work anymore with recent CPU / GCC / pthread (or some of them) #454

Open tbarbette opened 4 years ago

tbarbette commented 4 years ago

Hi all,

I started to see the Spinlock assertion failing at line : https://github.com/kohler/click/blob/0fc6c4e67f3cfc63f838b3a84bdb03728621b3df/include/click/sync.hh#L145

My guess is click_processor_t is a typedef of pthread_t and therefore is 64bit until "recently" (when? and not sure why I see it now. Either because I did not use much Spinlock before, or because we're with the latest CascadeLake CPU that does funny stuff).

Therefore I don't see how this could be atomic: https://github.com/kohler/click/blob/0fc6c4e67f3cfc63f838b3a84bdb03728621b3df/include/click/sync.hh#L148-L149 Or it's missing a barrier? Lock may be written before owner. Despite x86 is supposed to have in-order memory write, the compiler may do some weird stuff, especially that I see this behaviour with -O3 only.

In practice, using click_current_cpu_id() instead of pthread_self() to detect the current thread id, and therefore using a 32bit unsigned solves the issue (and makes it probably faster). But that could be only assignment luck and/or alignment luck.

Tom