HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Simple spinlock for C using ASM

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplespinlockasmusingfor

Problem

This is my second attempt to make simple lock using extended assembly.
See https://stackoverflow.com/questions/37241553/locks-around-memory-manipulation-via-inline-assembly/37246263

The code:

static inline void atomic_open(volatile int *gate)
{
    asm volatile (
        "jmp check\n"
        "wait:\n"                      // Set spawning point.
        "pause\n"                      // Stroke your beard. (If you have one.)
        "check:\n"
        "cmp %[open], %[gate]\n"       // Check if gate is open.
        "jne wait\n"                   // If it isn't open, wait. If it is open, go through the gate.
        "lock xadd %[lock], %[gate]\n" // In case you where not alone entering the gate, at least try to be the first one to put the lock.
        "cmp %[lock], %[open]\n"       // Check to see you where the first one who put the lock.
        // "If not, consider thug life..."
        "jne wait\n"                   // If you didn't win, respawn and try again later.
        : [gate] "=m" (*gate)
        : [lock] "r" (1), [open] "r" (0)
    );
}

static inline void atomic_close(volatile int *gate)
{
    asm volatile (
        "pause\n"
        "lock xchg %[lock], %[gate]\n"
        : [gate] "=m" (*gate)
        : [lock] "r" (0)
    );
}

// Usage, example.
volatile int atomic_gate_memory = 0;
void *mymalloc(size_t size)
{
    atomic_open(&atomic_gate_memory);
    void *ptr = malloc(size);
    atomic_close(&atomic_gate_memory);
    return ptr;
}


The question: Will atomic_[open/close] make mymalloc both threadsafe and reentrant?

  • If no, what is wrong?



  • If yes, it is still wrong; isn't it?... Give me a good rant about what to consider, what is missing or about better approach. If you want to suggest libraries, please restrict your self to C. I am not experienced enough to bind C++ stuff to other languages, so I often can't use the good stuff over there :'(

Solution

-
Op's code has a possible race condition between the lock and compare.

"lock xadd %[lock], %[gate]\n" 
"cmp %[lock], %[open]\n"


-
OP identified: lock is not certainly made a shared variable.

Code Snippets

"lock xadd %[lock], %[gate]\n" 
"cmp %[lock], %[open]\n"

Context

StackExchange Code Review Q#128523, answer score: 2

Revisions (0)

No revisions yet.