On Wed, Mar 03, 2010 at 09:23:46PM -0500, David Dillow wrote: > On Wed, 2010-03-03 at 11:55 +1100, Paul Mackerras wrote: > > Well, if the smp_wmb() is supposed to make the assignment to > > tp->intr_mask globally visible before any effects of the RTL_W16(), > > then it's buggy. But from the comments it appears that the smp_wmb() > > might be intended to order the store to tp->intr_mask with respect to > > following cacheable stores, rather than with respect to the RTL_W16(), > > which would be OK. I can't say without having a much closer look at > > what that driver is actually doing. > > It's buggy. The code was intended to ensure the write to intr_mask was > visible to other CPUs before we told the NIC that it could generate > another interrupt. Give the definition of the barriers above, this > should be wmb() instead of smp_wmb(). There's a whole bunch of other drivers doing exactly the same thing - just grep drivers/net for smp_wmb(). ;( -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
