svn commit: r368325 - in stable/12: lib/libc/sys lib/libthr/tests sys/kern
Kyle Evans
kevans at FreeBSD.org
Fri Dec 4 02:37:34 UTC 2020
Author: kevans
Date: Fri Dec 4 02:37:33 2020
New Revision: 368325
URL: https://svnweb.freebsd.org/changeset/base/368325
Log:
MFC r367742-r367743: _umtx_op: documentation and compat32 fix
r367742:
_umtx_op: document UMTX_OP_SEM2_WAIT copyout behavior
This clever technique to get a time remaining back was added to support
sem_clockwait_np.
Reviewed by: kib, vangyzen
Differential Revision: https://reviews.freebsd.org/D27160
r367743:
_umtx_op: fix a compat32 bug in UMTX_OP_NWAKE_PRIVATE
Specifically, if we're waking up some value n > BATCH_SIZE, then the
copyin(9) is wrong on the second iteration due to upp being the wrong type.
upp is currently a uint32_t**, so upp + pos advances it by twice as many
elements as it should (host pointer size vs. compat32 pointer size).
Fix it by just making upp a uint32_t*; it's still technically a double
pointer, but the distinction doesn't matter all that much here since we're
just doing arithmetic on it.
Add a test case that demonstrates the problem, placed with the libthr tests
since one messing with _umtx_op should be running these tests. Running under
compat32, the new test case will hang as threads after the first 128 get
missed in the wake. it's not immediately clear how to hit it in practice,
since pthread_cond_broadcast() uses a smaller (sleepq batch?) size observed
to be around ~50 -- I did not spend much time digging into it.
The uintptr_t change makes no functional difference, but i've tossed it in
since it's more accurate (semantically).
Added:
stable/12/lib/libthr/tests/umtx_op_test.c
- copied unchanged from r367743, head/lib/libthr/tests/umtx_op_test.c
Modified:
stable/12/lib/libc/sys/_umtx_op.2
stable/12/lib/libthr/tests/Makefile
stable/12/sys/kern/kern_umtx.c
Directory Properties:
stable/12/ (props changed)
Modified: stable/12/lib/libc/sys/_umtx_op.2
==============================================================================
--- stable/12/lib/libc/sys/_umtx_op.2 Fri Dec 4 02:28:45 2020 (r368324)
+++ stable/12/lib/libc/sys/_umtx_op.2 Fri Dec 4 02:37:33 2020 (r368325)
@@ -28,7 +28,7 @@
.\"
.\" $FreeBSD$
.\"
-.Dd November 13, 2017
+.Dd November 16, 2020
.Dt _UMTX_OP 2
.Os
.Sh NAME
@@ -1101,6 +1101,15 @@ The arguments to the request are:
.It Fa obj
Pointer to the semaphore (of type
.Vt struct _usem2 ) .
+.It Fa uaddr
+Size of the memory passed in via the
+.Fa uaddr2
+argument.
+.It Fa uaddr2
+Optional pointer to a structure of type
+.Vt struct _umtx_time ,
+which may be followed by a structure of type
+.Vt struct timespec .
.El
.Pp
Put the requesting thread onto a sleep queue if the semaphore counter
@@ -1124,6 +1133,18 @@ An unblocked signal delivered during such wait results
interruption and
.Er EINTR
error.
+.Pp
+If
+.Dv UMTX_ABSTIME
+was not set, and the operation was interrupted and the caller passed in a
+.Fa uaddr2
+large enough to hold a
+.Vt struct timespec
+following the initial
+.Vt struct _umtx_time ,
+then the
+.Vt struct timespec
+is updated to contain the unslept amount.
.It Dv UMTX_OP_SEM2_WAKE
Wake up waiters on semaphore lock.
The arguments to the request are:
Modified: stable/12/lib/libthr/tests/Makefile
==============================================================================
--- stable/12/lib/libthr/tests/Makefile Fri Dec 4 02:28:45 2020 (r368324)
+++ stable/12/lib/libthr/tests/Makefile Fri Dec 4 02:37:33 2020 (r368325)
@@ -33,6 +33,8 @@ NETBSD_ATF_TESTS_SH+= cancel_test
NETBSD_ATF_TESTS_SH+= exit_test
NETBSD_ATF_TESTS_SH+= resolv_test
+ATF_TESTS_C+= umtx_op_test
+
LIBADD+= pthread
LIBADD.fpu_test+= m
LIBADD.sem_test+= rt
Copied: stable/12/lib/libthr/tests/umtx_op_test.c (from r367743, head/lib/libthr/tests/umtx_op_test.c)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ stable/12/lib/libthr/tests/umtx_op_test.c Fri Dec 4 02:37:33 2020 (r368325, copy of r367743, head/lib/libthr/tests/umtx_op_test.c)
@@ -0,0 +1,108 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2020 Kyle Evans <kevans at FreeBSD.org>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include <sys/types.h>
+#include <sys/umtx.h>
+
+#include <pthread.h>
+
+#include <atf-c.h>
+
+/*
+ * This is an implementation detail of _umtx_op(2), pulled from
+ * sys/kern/kern_umtx.c. The relevant bug observed that requests above the
+ * batch side would not function as intended, so it's important that this
+ * reflects the BATCH_SIZE configured there.
+ */
+#define UMTX_OP_BATCH_SIZE 128
+#define THREAD_COUNT ((UMTX_OP_BATCH_SIZE * 3) / 2)
+
+static pthread_mutex_t static_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+static int batched_waiting;
+
+static void *
+batching_threadfunc(void *arg)
+{
+
+ pthread_mutex_lock(&static_mutex);
+ ++batched_waiting;
+ pthread_mutex_unlock(&static_mutex);
+ _umtx_op(arg, UMTX_OP_WAIT_UINT_PRIVATE, 0, NULL, NULL);
+
+ return (NULL);
+}
+
+ATF_TC(batching);
+ATF_TC_HEAD(batching, tc)
+{
+ atf_tc_set_md_var(tc, "descr",
+ "Checks batching of UMTX_OP_NWAKE_PRIVATE");
+}
+ATF_TC_BODY(batching, tc)
+{
+ uintptr_t addrs[THREAD_COUNT];
+ uint32_t vals[THREAD_COUNT];
+ pthread_t threads[THREAD_COUNT];
+
+ for (int i = 0; i < THREAD_COUNT; i++) {
+ addrs[i] = (uintptr_t)&vals[i];
+ vals[i] = 0;
+ pthread_create(&threads[i], NULL, batching_threadfunc,
+ &vals[i]);
+ }
+
+ pthread_mutex_lock(&static_mutex);
+ while (batched_waiting != THREAD_COUNT) {
+ pthread_mutex_unlock(&static_mutex);
+ pthread_yield();
+ pthread_mutex_lock(&static_mutex);
+ }
+
+ /*
+ * Spin for another .50 seconds to make sure they're all safely in the
+ * kernel.
+ */
+ usleep(500000);
+
+ pthread_mutex_unlock(&static_mutex);
+ _umtx_op(addrs, UMTX_OP_NWAKE_PRIVATE, THREAD_COUNT, NULL, NULL);
+
+ for (int i = 0; i < THREAD_COUNT; i++) {
+ ATF_REQUIRE_EQ(0, pthread_join(threads[i], NULL));
+ }
+}
+
+ATF_TP_ADD_TCS(tp)
+{
+
+ ATF_TP_ADD_TC(tp, batching);
+ return (atf_no_error());
+}
Modified: stable/12/sys/kern/kern_umtx.c
==============================================================================
--- stable/12/sys/kern/kern_umtx.c Fri Dec 4 02:28:45 2020 (r368324)
+++ stable/12/sys/kern/kern_umtx.c Fri Dec 4 02:37:33 2020 (r368325)
@@ -4360,10 +4360,10 @@ __umtx_op_sem2_wait_compat32(struct thread *td, struct
static int
__umtx_op_nwake_private32(struct thread *td, struct _umtx_op_args *uap)
{
- uint32_t uaddrs[BATCH_SIZE], **upp;
+ uint32_t uaddrs[BATCH_SIZE], *upp;
int count, error, i, pos, tocopy;
- upp = (uint32_t **)uap->obj;
+ upp = (uint32_t *)uap->obj;
error = 0;
for (count = uap->val, pos = 0; count > 0; count -= tocopy,
pos += tocopy) {
@@ -4372,7 +4372,7 @@ __umtx_op_nwake_private32(struct thread *td, struct _u
if (error != 0)
break;
for (i = 0; i < tocopy; ++i)
- kern_umtx_wake(td, (void *)(intptr_t)uaddrs[i],
+ kern_umtx_wake(td, (void *)(uintptr_t)uaddrs[i],
INT_MAX, 1);
maybe_yield();
}
More information about the svn-src-stable
mailing list