Fixes nasty CAS bug
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 17 May 2011 21:40:57 +0000 (14:40 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:36:03 +0000 (17:36 -0700)
This one would only show up if you had enough contention to cause CAS to
fail, at which point eax was clobbered, and if the number you were
working on was > 255, such as when you have 300-500 kthreads.  Goddamn.

kern/arch/i686/atomic.h
kern/src/testing.c
tests/block_test.c
user/parlib/include/i686/atomic.h

index 1bbdc30..5517203 100644 (file)
@@ -101,7 +101,7 @@ static inline bool atomic_add_not_zero(atomic_t *number, long val)
 static inline bool atomic_sub_and_test(atomic_t *number, long val)
 {
        bool b;
-       asm volatile("lock sub %2,%1; setz %0" : "=r"(b), "=m"(*number)
+       asm volatile("lock sub %2,%1; setz %0" : "=q"(b), "=m"(*number)
                                               : "r"(val), "m"(*number)
                                               : "cc" );
        return b;
@@ -120,11 +120,12 @@ static inline uint32_t atomic_swap(uint32_t *addr, uint32_t val)
        return val;
 }
 
-/* reusing exp_val for the bool return.  1 (TRUE) for success (like test). */
+/* reusing exp_val for the bool return.  1 (TRUE) for success (like test).  Need
+ * to zero eax, since it will get set if the cmpxchgl failed. */
 static inline bool atomic_comp_swap(uint32_t *addr, uint32_t exp_val,
                                     uint32_t new_val)
 {
-       asm volatile("lock cmpxchgl %4,%1; sete %%al"
+       asm volatile("lock cmpxchgl %4,%1; movl $0,%%eax; sete %%al"
                     : "=a"(exp_val), "=m"(*addr)
                     : "m"(*addr), "a"(exp_val), "r"(new_val)
                     : "cc");
index 0825f2f..884828a 100644 (file)
@@ -1248,3 +1248,84 @@ void test_kthreads(void)
        sleep_on(&sem);
        printk("Kthread restarted!, Stacktop is %08p.\n", get_stack_top());
 }
+
+/* Runs a simple test between core 0 (caller) and core 2 */
+void test_kref(void)
+{
+       struct kref local_kref;
+       bool done = FALSE;
+       /* Second player's kmsg */
+       void test_kref_2(struct trapframe *tf, uint32_t srcid, void *a0, void *a1,
+                        void *a2)
+       {
+               struct kref *kref = (struct kref*)a0;
+               bool *done = (bool*)a1;
+               enable_irq();
+               for (int i = 0; i < 10000000; i++) {
+                       kref_get(kref, 1);
+                       set_core_timer(1, TRUE);
+                       udelay(2);
+                       kref_put(kref);
+               }
+               *done = TRUE;
+       }
+       
+       kref_init(&local_kref, fake_release, 1);
+       send_kernel_message(2, test_kref_2, &local_kref, &done, 0, KMSG_ROUTINE);
+       for (int i = 0; i < 10000000; i++) {
+               kref_get(&local_kref, 1);
+               udelay(2);
+               kref_put(&local_kref);
+       }
+       while (!done)
+               cpu_relax();
+       assert(kref_refcnt(&local_kref) == 1);
+       printk("[TEST-KREF] Simple 2-core getting/putting passed.\n");
+}
+
+void test_atomics(void)
+{
+       /* subtract_and_test */
+       atomic_t num;
+       /* Test subing to 0 */
+       atomic_init(&num, 1);
+       assert(atomic_sub_and_test(&num, 1) == 1);
+       atomic_init(&num, 2);
+       assert(atomic_sub_and_test(&num, 2) == 1);
+       /* Test not getting to 0 */
+       atomic_init(&num, 1);
+       assert(atomic_sub_and_test(&num, 0) == 0);
+       atomic_init(&num, 2);
+       assert(atomic_sub_and_test(&num, 1) == 0);
+       /* Test negatives */
+       atomic_init(&num, -1);
+       assert(atomic_sub_and_test(&num, 1) == 0);
+       atomic_init(&num, -1);
+       assert(atomic_sub_and_test(&num, -1) == 1);
+       /* Test larger nums */
+       atomic_init(&num, 265);
+       assert(atomic_sub_and_test(&num, 265) == 1);
+       atomic_init(&num, 265);
+       assert(atomic_sub_and_test(&num, 2) == 0);
+
+       /* CAS */
+       /* Simple test, make sure the bool retval of CAS handles failure */
+       void test_cas_val(long init_val)
+       {
+               long actual_num, old_num;
+               int attempt;
+               actual_num = init_val;
+               attempt = 0;
+               do {
+                       old_num = actual_num;
+                       /* First time, try to fail */
+                       if (attempt == 0) 
+                               old_num++;
+                       attempt++;      
+               } while (!atomic_comp_swap((uint32_t*)&actual_num, old_num, old_num + 10));
+               if (actual_num != init_val + 10)
+                       printk("FUCK, CAS test failed for %d\n", init_val);
+       }
+       test_cas_val(257);
+       test_cas_val(1);
+}
index f790d38..3ed6315 100644 (file)
@@ -11,7 +11,7 @@ pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
        printf(__VA_ARGS__); \
        pthread_mutex_unlock(&lock);
 
-#define NUM_TEST_THREADS 20
+#define NUM_TEST_THREADS 500
 #define NUM_TEST_LOOPS 1000
 
 pthread_t my_threads[NUM_TEST_THREADS];
index e32ab6b..00f7fcc 100644 (file)
@@ -59,7 +59,7 @@ static inline uint32_t atomic_swap(uint32_t *addr, uint32_t val)
 static inline bool atomic_comp_swap(uint32_t *addr, uint32_t exp_val,
                                     uint32_t new_val)
 {
-       asm volatile("lock cmpxchgl %4,%1; sete %%al"
+       asm volatile("lock cmpxchgl %4,%1; movl $0,%%eax; sete %%al"
                     : "=a"(exp_val), "=m"(*addr)
                     : "m"(*addr), "a"(exp_val), "r"(new_val)
                     : "cc");