3

I am trying to understand the "unlocking an unlocked mutex" is not allowed will lead to unpredictable behavior w.r.t Linux kernel mutex, when i look at the code i do not see anything to this effect.

Specifically:

/**
 * try to promote the mutex from 0 to 1. if it wasn't 0, call <function>
 * In the failure case, this function is allowed to either set the value to
 * 1, or to set it to a value lower than one.
 * If the implementation sets it to a value of lower than one, the
 * __mutex_slowpath_needs_to_unlock() macro needs to return 1, it needs
 * to return 0 otherwise.
 */
static inline void
__mutex_fastpath_unlock(atomic_t *count, void (*fail_fn)(atomic_t *))
{
    if (unlikely(atomic_xchg(count, 1) != 0))
        fail_fn(count);
}

where fail_fn is

static inline void
__mutex_unlock_common_slowpath(atomic_t *lock_count, int nested)
{
    struct mutex *lock = container_of(lock_count, struct mutex, count);
    unsigned long flags;

    spin_lock_mutex(&lock->wait_lock, flags);
    mutex_release(&lock->dep_map, nested, _RET_IP_);
    debug_mutex_unlock(lock);

    /*
     * some architectures leave the lock unlocked in the fastpath failure
     * case, others need to leave it locked. In the later case we have to
     * unlock it here
     */
    if (__mutex_slowpath_needs_to_unlock())
        atomic_set(&lock->count, 1);

    if (!list_empty(&lock->wait_list)) {
        /* get the first entry from the wait-list: */
        struct mutex_waiter *waiter =
                list_entry(lock->wait_list.next,
                       struct mutex_waiter, list);

        debug_mutex_wake_waiter(lock, waiter);

        wake_up_process(waiter->task);
    }

    spin_unlock_mutex(&lock->wait_lock, flags);
}

From what i can tell if it is unlocked it will remain unlocked regardless of what __mutex_slowpath_needs_to_unlock() is, am i missing something here?

chrk
  • 4,037
  • 2
  • 39
  • 47
newbie
  • 41
  • 3
  • 3
    "Unpredictable behavior" can include "works without problems right now, but no promises regarding future versions". – Wyzard Aug 16 '14 at 03:07

1 Answers1

1

As suggested by Wyzard, the documentation may want to keep some ways open. For example, some other implementations can use atomic increment instead of the exchange, like this one:

static inline void __mutex_fastpath_unlock(atomic_t *v,
                                           void (*fail_fn)(atomic_t *))
{
        asm_volatile_goto(LOCK_PREFIX "   incl %0\n"
                          "   jg %l[exit]\n"
                          : : "m" (v->counter)
                          : "memory", "cc"
                          : exit);

As you can see, it is not safe to call it twice in a row since accompanying try_lock does not expect any other values except 0 and 1:

static inline int __mutex_fastpath_trylock(atomic_t *count,
                                           int (*fail_fn)(atomic_t *))
{
        if (likely(atomic_cmpxchg(count, 1, 0) == 1))
Anton
  • 6,349
  • 1
  • 25
  • 53