Shared Counter Gives Different Value With Spinlocks

Question

I have the following code:

#include <stdio.h>
#include <pthread.h>
#define THREAD_CNT 10
#define ITER 100
#define PRINT 1

int lock;
unsigned long long int counter;

void spin_lock(int *p) {
    while(!__sync_bool_compare_and_swap(p, 0, 1));
}

void spin_unlock(int volatile *p) {
    asm volatile ("");
    *p = 0;
}

void *exerciser(void *arg) {
    unsigned long long int i;
    int id = (int)arg;
    for(i = 0; i < ITER; i++) {
        spin_lock(&lock);
        counter = counter + 1;
        if(PRINT) {
            printf("%d: Incrementing counter: %llu -> %llu\n", id, counter-1, counter);
        }
        spin_unlock(&lock);
    }
    pthread_exit(NULL);
}

int main(int argc, char *argv[]) {
    pthread_t thread[THREAD_CNT];
    counter = 0;
    int i;
    for(i = 0; i < THREAD_CNT; i++) {
        pthread_create(&thread[i], NULL, exerciser, (void *) i);
    }
    for(i = 0; i < THREAD_CNT; i++) {
        pthread_join(thread[i], NULL);
    }
    printf("Sum: %llu\n", counter);
    printf("Main: Program completed. Exiting.\n");
    pthread_exit(NULL);
}

When PRINT is defined as 1, I get the correct counter value at the end:

7: Incrementing counter: 996 -> 997
7: Incrementing counter: 997 -> 998
7: Incrementing counter: 998 -> 999
7: Incrementing counter: 999 -> 1000
Sum: 1000
Main: Program completed. Exiting.

If I make PRINT 0, I get the following (multiple runs):

$ ./a.out
Sum: 991
Main: Program completed. Exiting.
$ ./a.out 
Sum: 1000
Main: Program completed. Exiting.
$ ./a.out 
Sum: 962
Main: Program completed. Exiting.
$ ./a.out 
Sum: 938
Main: Program completed. Exiting.

Any insight as to what's going on? Why is it that when I have the print statement enabled my results are (consistently) correct, but I disable it and my counter doesn't reach the target value? I've used pthread quite a bit but not very experienced with using spinlocks directly.

Any help or feedback is appreciated.

Answer 1

Your lock methods aren't actually doing anything: since arguments are passed by value, you're not actually testing/setting the value of your lock global. You're only changing the value of the copy of the variable your function gets.

If instead your spin_lock/spin_unlock methods took a pointer to the integer (i.e. &lock) to use then your code should work.

Your load bearing printf probably helps by causing some unintended synchonisation because printf is supposed to be threadsafe.