{"id":15,"date":"2007-05-13T08:36:01","date_gmt":"2007-05-13T16:36:01","guid":{"rendered":"http:\/\/digitalvampire.org\/blog\/index.php\/2007\/05\/13\/atomic-cargo-cults\/"},"modified":"2017-07-20T00:19:28","modified_gmt":"2017-07-20T08:19:28","slug":"atomic-cargo-cults","status":"publish","type":"post","link":"https:\/\/digitalvampire.org\/blog\/index.php\/2007\/05\/13\/atomic-cargo-cults\/","title":{"rendered":"Atomic cargo cults"},"content":{"rendered":"

Cargo cult programming<\/a> refers to “ritual inclusion of code or program structures that serve no real purpose.” One annoying example of this that I see a lot in kernel code that I review is the inappropriate use of atomic_t<\/tt>, in the belief that it magically wards off races.<\/p>\n

This type of bogosity is usually marked by variables or structure members of type atomic_t<\/tt>, which are only ever accessed through atomic_read()<\/tt> and atomic_set()<\/tt> without ever using a real atomic operation such as atomic_inc()<\/tt> or atomic_dec_and_test()<\/tt>. Such programming reaches its apotheosis in code like:<\/p>\n

\r\n        atomic_set(&head, (atomic_read(&head) + 1) % size);<\/pre>\n
(and yes, this is essentially real code, although I’ve paraphrased it to protect the guilty from embarrassment).<\/p>\n
The only point of atomic_t<\/tt> is that arithmetic operations (like atomic_inc()<\/tt> et al) are atomic<\/em> in the sense that there is no window where two racing operations can step on each other.  This helps with situations where an int<\/tt> variable might have ++i<\/tt> and --i<\/tt> race with each other; since these operations are probably implemented as read-modify-write.  So if i<\/tt> starts out as 0, the value of i<\/tt> after the two operations might be 0, -1 or 1 depending on when each operation reads the value of i<\/tt> and what order they write back the final value.<\/p>\n
If you never use any of these atomic operations, then there’s no point in using atomic_set()<\/tt> to assign a variable and atomic_read()<\/tt> to read it.  Your code is no more or less safe than if would be just using normal int<\/tt> variables and assignments.<\/p>\n
One way to think about atomic operations is that they might be an optimization instead of using a lock to protect access to a variable.  Unfortunately I’m not sure if this will help anyone get things right, since I also see plenty of code like:<\/p>\n
\r\nint x;\r\n\r\nint foo(void)\r\n{\r\n\r\n        int y;\r\n\r\n        spin_lock(&lock);\r\n        y = x;\r\n        spin_unlock(&lock);\r\n\r\n        return y;\r\n}<\/pre>\n
which is the analogous cargo-cult anti-pattern for spinlocks.<\/p>\n
Maybe the best rule would be: if the only functions using atomic_t<\/tt> in your code are atomic_set()<\/tt> and atomic_read()<\/tt>, then you need to write a comment explaining why you’re using atomic at all.  Since the vast majority of the time, such a comment will be impossible to write, maybe this will cut down on cargo cult programming a bit.  Or more likely it will just make code review more fun by generating nonsensical comments for me to chuckle at.<\/p>\n","protected":false},"excerpt":{"rendered":"
Cargo cult programming refers to “ritual inclusion of code or program structures that serve no real purpose.” One annoying example of this […]<\/p>\n","protected":false},"author":2,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[4],"tags":[],"class_list":["post-15","post","type-post","status-publish","format-standard","hentry","category-hacking"],"_links":{"self":[{"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/posts\/15","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/users\/2"}],"replies":[{"embeddable":true,"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/comments?post=15"}],"version-history":[{"count":1,"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/posts\/15\/revisions"}],"predecessor-version":[{"id":647,"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/posts\/15\/revisions\/647"}],"wp:attachment":[{"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/media?parent=15"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/categories?post=15"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/tags?post=15"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}