Hello andreip, nicolasroard,

I'd like you to do a code review.  Please execute
        g4 diff -c 8906467

or point your web browser to
        http://mondrian/8906467
(this changelist has been uploaded to Mondrian)

to review the following code:

Change 8906467 by [EMAIL PROTECTED] on 2008/11/07 12:49:52 *pending*

        Add Linux/Android ARM atomic operations.
        
        This implements AtomicIncrement using the Linux Kernel atomic
        exchange helper. This is at a fixed, stable, magic address
        (0xffff0fc0). The other atomic operations functions are not
        implemented as they are unused and untested.
        
        This also adds OS_ANDROID to the users of atomic_ops_linux.h
        as it is compatible.
        
        PRESUBMIT=passed
        R=andreip,nicolasroard
        [EMAIL PROTECTED]
        DELTA=50  (49 added, 0 deleted, 1 changed)
        OCL=8906467

Affected files ...

... //depot/googleclient/gears/opensource/gears/base/common/atomic_ops.h#1 edit
... 
//depot/googleclient/gears/opensource/gears/base/common/atomic_ops_linux.h#1 
edit

50 delta lines: 49 added, 0 deleted, 1 changed

Also consider running:
        g4 lint -c 8906467

which verifies that the changelist doesn't introduce new style violations.

If you can't do the review, please let me know as soon as possible.  During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately.  Visit
http://www/eng/code_review.html for more information.

This is a semiautomated message from "g4 mail".  Complaints or suggestions?
Mail [EMAIL PROTECTED]
Change 8906467 by [EMAIL PROTECTED] on 2008/11/07 12:49:52 *pending*

        Add Linux/Android ARM atomic operations.
        
        This implements AtomicIncrement using the Linux Kernel atomic
        exchange helper. This is at a fixed, stable, magic address
        (0xffff0fc0). The other atomic operations functions are not
        implemented as they are unused and untested.
        
        This also adds OS_ANDROID to the users of atomic_ops_linux.h
        as it is compatible.

Affected files ...

... //depot/googleclient/gears/opensource/gears/base/common/atomic_ops.h#1 edit
... 
//depot/googleclient/gears/opensource/gears/base/common/atomic_ops_linux.h#1 
edit

==== //depot/googleclient/gears/opensource/gears/base/common/atomic_ops.h#1 - 
/usr/local/google/home/jripley/gears-trunk2/googleclient/gears/opensource/gears/base/common/atomic_ops.h
 ====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/atomic_ops.h        
2008-11-07 12:49:58.000000000 +0000
+++ googleclient/gears/opensource/gears/base/common/atomic_ops.h        
2008-11-07 12:37:29.000000000 +0000
@@ -37,7 +37,7 @@
 
 #if defined(OS_MACOSX)
 #include "gears/base/common/atomic_ops_osx.h"
-#elif defined(LINUX)
+#elif defined(LINUX) || defined(OS_ANDROID)
 #include "gears/base/common/atomic_ops_linux.h"
 #elif defined(WIN32)
 #include "gears/base/common/atomic_ops_win32.h"
==== 
//depot/googleclient/gears/opensource/gears/base/common/atomic_ops_linux.h#1 - 
/usr/local/google/home/jripley/gears-trunk2/googleclient/gears/opensource/gears/base/common/atomic_ops_linux.h
 ====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/atomic_ops_linux.h  
2008-11-07 12:49:59.000000000 +0000
+++ googleclient/gears/opensource/gears/base/common/atomic_ops_linux.h  
2008-11-07 12:43:26.000000000 +0000
@@ -30,6 +30,9 @@
 #define GEARS_BASE_COMMON_ATOMIC_OPS_LINUX_H__
 
 #include <stdint.h>
+
+#if defined(__i386__) || defined(__x86_64__)
+// Atomic operations for x86 CPU variants on Linux.
 
 typedef intptr_t AtomicWord;
 typedef int32_t Atomic32;
@@ -113,4 +116,50 @@
 
 #undef ATOMICOPS_COMPILER_BARRIER
 
+#elif defined(__arm__)
+// Atomic operations for ARM CPU variants on Linux. Only
+// AtomicIncrement is implemented as the other functions are unused.
+
+typedef int AtomicWord;
+
+typedef AtomicWord (*LinuxKernelCmpxchgFunc)(AtomicWord old_value,
+                                             AtomicWord new_value,
+                                             volatile AtomicWord* ptr);
+
+// Call the magic userland-kernel bridge address to perform an atomic
+// compare-exchange. Returns zero if the exchange was performed, or
+// non-zero if the exchange was not performed.
+inline int LinuxKernelCmpxchg(AtomicWord old_value,
+                              AtomicWord new_value,
+                              volatile AtomicWord* ptr) {
+  // 0xffff0fc0 is the hard coded address of a function provided by
+  // the kernel which implements an atomic compare-exchange. On older
+  // ARM architecture revisions (pre-v6) this may be implemented using
+  // a syscall. This address is stable, and in active use (hard coded)
+  // by at least glibc-2.7 and the Android C library.
+  return reinterpret_cast<LinuxKernelCmpxchgFunc>(0xffff0fc0)(old_value,
+                                                              new_value,
+                                                              ptr);
+}
+
+inline AtomicWord AtomicIncrement(volatile AtomicWord* ptr,
+                                  AtomicWord increment) {
+  for (;;) {
+    // Atomic exchange the old value with an incremented one.
+    AtomicWord old_value = *ptr;
+    AtomicWord new_value = old_value + increment;
+    if (LinuxKernelCmpxchg(old_value, new_value, ptr) == 0) {
+      // The exchange took place as expected.
+      return new_value;
+    }
+    // Otherwise, *ptr changed mid-loop and we need to retry.
+  }
+}
+
+#else
+
+#error "Unsupported CPU for Linux atomic operations."
+
+#endif
+
 #endif  // GEARS_BASE_COMMON_ATOMIC_OPS_LINUX_H__

Reply via email to