{"id":17,"date":"2007-06-05T19:17:06","date_gmt":"2007-06-06T03:17:06","guid":{"rendered":"http:\/\/digitalvampire.org\/blog\/index.php\/2007\/06\/05\/17\/"},"modified":"2007-06-06T04:56:07","modified_gmt":"2007-06-06T12:56:07","slug":"17","status":"publish","type":"post","link":"https:\/\/digitalvampire.org\/blog\/index.php\/2007\/06\/05\/17\/","title":{"rendered":"If I’m a crusader, where’s my cape?"},"content":{"rendered":"

Pete Zaitcev replied<\/a> to my earlier post<\/a> about misuse of atomic variables. I never really thought of myself as a crusader or as particularly quixotic, but I’ll respond to the technical content of Pete’s post. I think the disagreement was with my disparagement of the anti-pattern:<\/p>\n

\r\nint x;\r\n\r\nint foo(void)\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
Pete is entirely correct that spin_lock()<\/tt> and spin_unlock()<\/tt> have full memory barriers; otherwise it would be impossible for anyone to use spinlocks correctly (and of course there’s still mmiowb()<\/tt> to trip you up when someone runs your driver on a big SGI machine).<\/p>\n
However, I still think that using a spinlock around an assignment that’s atomic anyway is at best pretty silly.  If you just need a memory barrier, then put an explicit mb()<\/tt> (or wmb()<\/tt> or rmb()<\/tt>) there, along with a fat comment about why you need to mess with memory barriers anyway.<\/p>\n
Also, Pete is not entirely correct when he says that atomic operations lack memory barriers.  All atomic operations that return a value (eg atomic_dec_and_test()<\/tt>) do have a full memory barrier, and if you want a barrier to go with an atomic operation that doesn’t return a value, such as atomic_inc()<\/tt>, the kernel does supply a full range of primitives such as smp_mb__after_atomic_inc()<\/tt>.  The file Documentation\/memory-barriers.txt<\/tt> in the kernel source tree explains all of this in excruciating detail.<\/p>\n
In the end the cost of an atomic op is roughly the same as the cost of a spin_lock()<\/tt>\/spin_unlock()<\/tt> pair (they both have to do one locked operation, and everything else is pretty much in the noise for all but the most performance criticial code).  Spinlocks are usually easier to think about, so I recommend only using atomic_t<\/tt> when it fits perfectly, such as a reference count (and even then using struct kref<\/tt> is probably better if you can).  I’ve found from doing code review that code using atomic_t<\/tt> almost always has bugs, and we don’t have any magic debugging tools to find them (the way we have CONFIG_PROVE_LOCKING<\/tt>, CONFIG_DEBUG_SPINLOCK_SLEEP<\/tt> and so on for locks).<\/p>\n
By the way, what’s up with mark mazurek<\/a>?  His blog seems to be an exact copy taken from Pete’s blog feed, with the added bonus of adding a comment to the post in my blog that Pete linked to.  There are no ads or really anything beyond an exact duplicate of Pete’s blog, so I can’t figure out what the angle is.<\/p>\n","protected":false},"excerpt":{"rendered":"
Pete Zaitcev replied to my earlier post about misuse of atomic variables. I never really thought of myself as a crusader or as particularly quixotic, but I’ll respond to the technical content of Pete’s post. I think the disagreement was with my disparagement of the anti-pattern: int x; int foo(void) { int y; spin_lock(&lock); y […]<\/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-17","post","type-post","status-publish","format-standard","hentry","category-hacking"],"_links":{"self":[{"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/posts\/17","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=17"}],"version-history":[{"count":0,"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/posts\/17\/revisions"}],"wp:attachment":[{"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/media?parent=17"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/categories?post=17"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/digitalvampire.org\/blog\/index.php\/wp-json\/wp\/v2\/tags?post=17"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}