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
F170641792: D59338.diff
Sat, Sep 5, 6:44 PM
F170489891: D59338.id185807.diff
Sat, Sep 5, 1:56 AM
F170489753: D59338.id185695.diff
Sat, Sep 5, 1:55 AM
F170489616: D59338.id185702.diff
Sat, Sep 5, 1:54 AM
F170487370: D59338.diff
Sat, Sep 5, 1:38 AM
Unknown Object (File)
Fri, Sep 4, 9:47 PM
Unknown Object (File)
Fri, Sep 4, 7:56 AM
Unknown Object (File)
Fri, Sep 4, 2:28 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 Not Applicable
Unit
Tests Not Applicable

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.