2010/11/1 Björn Smedman <bjorn.smedman@xxxxxxxxxxx>: > On Fri, Oct 29, 2010 at 2:57 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxx> wrote: > [snip] >> I'd rather not add it if its not fixing anything. I hate code that is >> there just because we have a fuzzy feeling it helps. >> >> So NACK unless it fixes something. >> >> Luis > > IMHO there is a right and wrong, at the very least inside a computer. > If we can't read the documentation (Documentation/memory-barriers.txt) > and decide if the patch is correct or incorrect (I'm not saying I can > with 100% certainty) then perhaps we should ask somebody who can? For what it's worth, I agree that there should be a memory barrier between updating the contents of descriptors and linking them into the linked list seen by the hardware. If I may quote dma-mapping.txt: "Consistent DMA memory does not preclude the usage of proper memory barriers. The CPU may reorder stores to consistent memory just as it may normal memory." .. and then it gives an example which is exactly this case. I had proposed such a patch for ath5k a while ago, but I never pushed it upstream since it didn't seem to help any of the synchronization problems we were seeing on the platforms for which we have hardware, while better locking did. If anyone in PPC land wants to see if an eieio helps them let me know :) One thing such a patch _does_ need, though, is a comment that describes why there is a barrier. Otherwise when people reorganize the code, they may forget to take the barrier with it, and it also lets late-comers know which data is serialized by the barrier. In the best case, said late-comers are more knowledgeable than me and fix the crap code that I write. -- Bob Copeland %% www.bobcopeland.com -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
