3
\$\begingroup\$

Revision #1 for Simple spinlock for C using ASM

The code:

static inline void atomic_open(volatile int *gate)
{
    asm volatile (
        "jmp check\n"                // Renegade selected, lets skip the line!
        "wait:\n"                    // Honest citizens wait in line.
        "pause\n"                    // Stroke beard, check phone/watch.
        "check:\n"                   // Ok, lets do this...
        "mov %[lock], %%eax\n"       // eax = 1
        "lock xchg %%eax, %[gate]\n" // Exhange eax with gate value.
        "test %%eax, %%eax\n"        // 1 = closed, 0 = open.
        "jnz wait\n"                 // Ohhh man, here I go again...
        : [gate] "=m" (*gate)
        : [lock] "r" (1)
        : "eax"                      // Tell compiler you want to use eax register.
    );

}

static inline void atomic_close(volatile int *gate)
{
    asm volatile (
        "mov %[unlock], %[gate]\n"
        : [gate] "=m" (*gate)
        : [unlock] "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 is the same as before: 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 :'(
\$\endgroup\$
1
\$\begingroup\$

As you have no doubt noticed, given your follow up post, whilst your current lock is thread safe, it isn't reentrant.

If you perform a double call to atomic_open from the same thread, the second call will fail to enter the gate (because it's already locked) and become deadlocked against itself.

lock xchg %%eax, %[gate]\n" // Exhange eax with gate value.
"test %%eax, %%eax\n"        // 1 = closed, 0 = open.

This is because there's no tracking of which thread opened the gate.

Not checking the thread has the potential to cause another issue (which you might not care about, because it's pretty common with spinlocks) whereby if you have a bug in your code, it is possible for a thread that doesn't own the lock to unlock it (anybody can call atomic_close and it will unlock, even if they didn't call atomic_open first).

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.