Rafal Jaworowski wrote:
On 2010-02-20, at 17:13, Nathan Whitehorn wrote:
Author: nwhitehorn
Date: Sat Feb 20 16:13:43 2010
New Revision: 204126
URL: http://svn.freebsd.org/changeset/base/204126
Log:
Merge r198724 to Book-E. casuword() non-atomically read the current value
of its argument before atomically replacing it, which could occasionally
return the wrong value on an SMP system. This resulted in user mutex
operations hanging when using threaded applications.
Have you got a particular test case when this was breaking, so I can test?
This typically shows up with heavy lock contention on umtx operations. I
discovered this because running csup died for me 100% of the time on my
Xserve, by hanging forever in some umtx code.
This change explicitly preserves the semantics of casuword -- it is just
the code for atomic_cmpset_32 copied from atomic.h, but returning the
value loading with lwarx, instead of replacing it with a success code.
This closes a race between val = *addr and atomic_cmpset. With the old
code, another CPU could change the value at addr between val = *addr and
atomic_cmpset, causing casuword to return the wrong value.
Modified:
head/sys/powerpc/booke/copyinout.c
Modified: head/sys/powerpc/booke/copyinout.c
==============================================================================
--- head/sys/powerpc/booke/copyinout.c Sat Feb 20 16:12:37 2010
(r204125)
+++ head/sys/powerpc/booke/copyinout.c Sat Feb 20 16:13:43 2010
(r204126)
@@ -295,8 +295,19 @@ casuword(volatile u_long *addr, u_long o
return (EFAULT);
}
- val = *addr;
- (void) atomic_cmpset_32((volatile uint32_t *)addr, old, new);
+ __asm __volatile (
+ "1:\tlwarx %0, 0, %2\n\t" /* load old value */
+ "cmplw %3, %0\n\t" /* compare */
+ "bne 2f\n\t" /* exit if not equal */
+ "stwcx. %4, 0, %2\n\t" /* attempt to store */
+ "bne- 1b\n\t" /* spin if failed */
+ "b 3f\n\t" /* we've succeeded */
+ "2:\n\t"
+ "stwcx. %0, 0, %2\n\t" /* clear reservation (74xx) */
The 74xx comment reference is somewhat confusing as the clear reservation
operation is pretty uniform accross 32-bit PowerPC I guess, and not 74xx
specific.
That's true. It should also be updated in atomic.h.
-Nathan
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
