Page MenuHomeFreeBSD

umtx: do not sleep on an unowned mutex after a spurious CAS failure
ClosedPublic

Authored by pkubaj on Thu, Sep 3, 12:01 PM.
Tags
None
Referenced Files
F170839439: D59338.diff
Sun, Sep 6, 11:20 PM
Unknown Object (File)
Sat, Sep 5, 6:44 PM
Unknown Object (File)
Sat, Sep 5, 1:56 AM
Unknown Object (File)
Sat, Sep 5, 1:55 AM
Unknown Object (File)
Sat, Sep 5, 1:54 AM
Unknown Object (File)
Sat, Sep 5, 1:38 AM
Unknown Object (File)
Fri, Sep 4, 9:47 PM
Unknown Object (File)
Fri, Sep 4, 7:56 AM
Subscribers

Details

Summary

On ll/sc architectures casueword32() may report a spurious
store-conditional failure (reservation lost to an interrupt, preemption,
or another CPU touching the same reservation granule), and this is
indistinguishable from a genuine comparison mismatch: both return 1.
That is intentional since D20772 and documented in casueword(9) ("The
store can fail on load-linked/store-conditional architectures."), so
callers must cope.

do_lock_normal() does not fully cope. When the initial
UMUTEX_UNOWNED -> id acquire CAS fails spuriously, the observed owner is
still UMUTEX_UNOWNED, so neither the UMUTEX_CONTESTED branch nor the
real-owner case applies, and execution falls through past the "rv == 1
but not contested, likely store failure" comment into the sleep path.
There, the contested-bit CAS (expecting the observed owner, i.e.
UMUTEX_UNOWNED) succeeds because the mutex really is unowned, stamping
m_owner = UMUTEX_CONTESTED with no owner tid, and the thread sleeps on
"umtxn" forever: nobody owns the mutex, so no unlock and no wakeup ever
arrive. In _UMUTEX_TRY mode the same situation returns a false EBUSY
for a free mutex.

Treat an observed owner of UMUTEX_UNOWNED like UMUTEX_CONTESTED: try to
acquire the mutex, setting the contested bit, instead of falling through
to the sleep path. rv == 1 with the observed value equal to the
expected value can only mean a spurious store failure, so the mutex is
free. If the acquire CAS fails again, the outer loop restarts and
re-evaluates ownership. The contested bit set with no waiters present
only costs the matching unlock one trip through the kernel.

This was hit in practice on powerpc64le (POWER9): the Swift runtime's
Synchronization.Mutex issues _umtx_op(UMTX_OP_MUTEX_LOCK) directly with
no userspace fast path, so an uncontended lock of an unowned mutex runs
the kernel CAS exactly where a spurious failure deadlocks
(single-threaded process parked on "umtxn" with m_owner == 0x80000000,
observed as Foundation.Process.run() hanging). libthr mostly masks the
bug because pthread_mutex_lock() enters the kernel only when there is a
real owner that will eventually issue a wakeup.

The mechanism was confirmed with an experimental powerpc kernel that
instead retried the ll/sc sequence inside casueword32()/casueword();
that also eliminated the hang, but is not proposed here since the
single-attempt semantics of casueword(9) are intentional.

MFC after: 2 weeks

Test Plan

Reproducer (Swift-free): 7 threads hammer a word sharing the umutex's
128-byte reservation granule while 1 thread loops
_umtx_op(UMTX_OP_MUTEX_LOCK/UNLOCK). On an unpatched powerpc64le POWER9
kernel (15.1-RELEASE and CURRENT) the lock thread parks in umtxq_sleep
("umtxn") with m_owner == 0x80000000 within ~1 second and never returns;
procstat -kk shows do_lock_umutex -> umtxq_sleep.

#include <sys/types.h>
#include <sys/umtx.h>
#include <pthread.h>
#include <string.h>
#include <stdio.h>
#include <stdint.h>
#include <unistd.h>

static struct { struct umutex m; volatile uint32_t nb; } s __attribute__((aligned(128)));
static volatile long iters = 0;
static volatile int done = 0;
static void *hammer(void *a){ while(!done){ s.nb++; } return 0; }

int main(void){
    pthread_t t[8];
    memset(&s,0,sizeof s);
    for(int i=0;i<7;i++) pthread_create(&t[i],0,hammer,0);
    for(long i=0;i<50000000;i++){
        iters=i;
        if(_umtx_op(&s.m, UMTX_OP_MUTEX_LOCK, 0, NULL, NULL)!=0) continue;
        _umtx_op(&s.m, UMTX_OP_MUTEX_UNLOCK, 0, NULL, NULL);
    }
    done=1;
    printf("completed all iters, no hang\n");
    return 0;
}

Compile-tested on powerpc64le (GENERIC64LE). A kernel carrying the
equivalent retry inside the powerpc casueword primitives was
runtime-validated on the same machine (reproducer runs to completion,
Swift Process.run() works); I will follow up here once a kernel with
this MI variant has been exercised against the reproducer as well.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Runtime-validated on the POWER9 (radix MMU) machine that originally reproduced the hang. With this change applied to releng/15.1 n283611 (GENERIC64LE), the reproducer from the test plan runs to completion — "completed all iters, no hang" after 50M lock/unlock iterations against 7 reservation-stealing threads — where the unpatched kernel parks in "umtxn" with m_owner == 0x80000000 within a second. The Swift Foundation.Process.run() test that originally exposed the bug also passes on the patched kernel.

sys/kern/kern_umtx.c
1432–1433

Is it enough (and more logical IMO) to change this line to owner == UMUTEX_CONTESTED || owner == UMUTEX_UNOWNED?

If yes, the assert at rv == 0 case also needs adjustment.

pkubaj edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Thu, Sep 3, 2:33 PM

The updated diff (widened branch, CAS expecting the observed owner, widened MPASS) is runtime-validated on the same POWER9 releng/15.1 machine: the reproducer completes all 50M iterations ("completed all iters, no hang") and Swift Foundation.Process.run() works. Same results as the previous version of the diff.