On Thu, Nov 22, 2018 at 06:56:32PM +0800, Gao Xiang wrote: > Hi Greg, > > On 2018/11/22 18:22, Greg Kroah-Hartman wrote: > > Please document this memory barrier. It does not make much sense to > > me... > > Because we need to make the other observers noticing the latest values modified > in this locking period before unfreezing the whole workgroup, one way is to use > a memory barrier and the other way is to use ACQUIRE and RELEASE. we selected > the first one. > > Hmmm...ok, I will add a simple message to explain this, but I think that is > plain enough for a lock... Sympathizing with Greg's request, let me add some specific suggestions: 1. It wouldn't hurt to indicate a pair of memory accesses which are intended to be "ordered" by the memory barrier in question (yes, this pair might not be unique, but you should be able to provide an example). 2. Memory barriers always come matched by other memory barriers, or dependencies (it really does not make sense to talk about a full barrier "in isolation"): please also indicate (an instance of) a matching barrier or the matching barriers. 3. How do the hardware threads communicate? In the acquire/release pattern you mentioned above, the load-acquire *reads from* a/the previous store-release, a memory access that follows the acquire somehow communicate with a memory access preceding the release... 4. It is a good practice to include the above information within an (inline) comment accompanying the added memory barrier (in fact, IIRC, checkpatch.pl gives you a "memory barrier without comment" warning when you omit to do so); not just in the commit message. Hope this helps. Please let me know if something I wrote is unclear, Andrea > > Thanks, > Gao Xiang > > > > > thanks, > > > > greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel