diff mbox series

[v2,20/22] selftests: futex: Test sys_futex_waitv() timeout

Message ID 20210923171111.300673-21-andrealmeid@collabora.com
State Not applicable
Headers
Series futex: splitup and waitv syscall |

Checks

Context Check Description
dj/TryBot-apply_patch fail Patch failed to apply to master at the time it was sent

Commit Message

André Almeida Sept. 23, 2021, 5:11 p.m. UTC 
  Test if the futex_waitv timeout is working as expected, using the
supported clockid options.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 .../futex/functional/futex_wait_timeout.c     | 21 ++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Vasily Gorbik Nov. 9, 2021, 11:18 a.m. UTC | #1 
On Thu, Sep 23, 2021 at 02:11:09PM -0300, André Almeida wrote:
> Test if the futex_waitv timeout is working as expected, using the
> supported clockid options.

> +	/* futex_waitv with CLOCK_MONOTONIC */
> +	if (futex_get_abs_timeout(CLOCK_MONOTONIC, &to, timeout_ns))
> +		return RET_FAIL;
> +	res = futex_waitv(&waitv, 1, 0, &to, CLOCK_MONOTONIC);
> +	test_timeout(res, &ret, "futex_waitv monotonic", ETIMEDOUT);
> +
> +	/* futex_waitv with CLOCK_REALTIME */
> +	if (futex_get_abs_timeout(CLOCK_REALTIME, &to, timeout_ns))
> +		return RET_FAIL;
> +	res = futex_waitv(&waitv, 1, 0, &to, CLOCK_REALTIME);
> +	test_timeout(res, &ret, "futex_waitv realtime", ETIMEDOUT);

Hi André,

when built with -m32 and run as compat this two futex_waitv calls hang
on x86 and s390 (noticed while wiring up futex_waitv). The rest of the
futex selftests pass. This suggests some common compat issue? Any ideas?

diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
index 5cc38de9d8ea..ddcb597d13ea 100644
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 INCLUDES := -I../include -I../../ -I../../../../../usr/include/ \
        -I$(KBUILD_OUTPUT)/kselftest/usr/include
-CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE -pthread $(INCLUDES)
+CFLAGS := $(CFLAGS) -g -m32 -O2 -Wall -D_GNU_SOURCE -pthread $(INCLUDES)
 LDLIBS := -lpthread -lrt
 
 HEADERS := \
--
André Almeida Nov. 9, 2021, 12:52 p.m. UTC | #2 
Hi Vasily,

Às 08:18 de 09/11/21, Vasily Gorbik escreveu:
> On Thu, Sep 23, 2021 at 02:11:09PM -0300, André Almeida wrote:
>> Test if the futex_waitv timeout is working as expected, using the
>> supported clockid options.
> 
>> +	/* futex_waitv with CLOCK_MONOTONIC */
>> +	if (futex_get_abs_timeout(CLOCK_MONOTONIC, &to, timeout_ns))
>> +		return RET_FAIL;
>> +	res = futex_waitv(&waitv, 1, 0, &to, CLOCK_MONOTONIC);
>> +	test_timeout(res, &ret, "futex_waitv monotonic", ETIMEDOUT);
>> +
>> +	/* futex_waitv with CLOCK_REALTIME */
>> +	if (futex_get_abs_timeout(CLOCK_REALTIME, &to, timeout_ns))
>> +		return RET_FAIL;
>> +	res = futex_waitv(&waitv, 1, 0, &to, CLOCK_REALTIME);
>> +	test_timeout(res, &ret, "futex_waitv realtime", ETIMEDOUT);
> 
> Hi André,
> 
> when built with -m32 and run as compat this two futex_waitv calls hang
> on x86 and s390 (noticed while wiring up futex_waitv). The rest of the
> futex selftests pass. This suggests some common compat issue? Any ideas?

The issue is that futex_waitv() only accepts struct timespec that uses
64bit members. When using -m32, glibc will give you a 32bit timespec,
thus the timeout won't wort. Someday glibc will provide 64bit timespec
to 32bit userspace, given that this is affected by y2038 bug.

In previous submissions I added a workaround for that in the
selftest[0]. Search for "Y2038 section for 32-bit applications" in that
link. I'll submit something like that for futex_waitv() timeout test.

[0]
https://lore.kernel.org/lkml/20210709001328.329716-6-andrealmeid@collabora.com/
Adhemerval Zanella Netto Nov. 9, 2021, 1 p.m. UTC | #3 
On 09/11/2021 09:52, André Almeida via Libc-alpha wrote:
> Hi Vasily,
> 
> Às 08:18 de 09/11/21, Vasily Gorbik escreveu:
>> On Thu, Sep 23, 2021 at 02:11:09PM -0300, André Almeida wrote:
>>> Test if the futex_waitv timeout is working as expected, using the
>>> supported clockid options.
>>
>>> +	/* futex_waitv with CLOCK_MONOTONIC */
>>> +	if (futex_get_abs_timeout(CLOCK_MONOTONIC, &to, timeout_ns))
>>> +		return RET_FAIL;
>>> +	res = futex_waitv(&waitv, 1, 0, &to, CLOCK_MONOTONIC);
>>> +	test_timeout(res, &ret, "futex_waitv monotonic", ETIMEDOUT);
>>> +
>>> +	/* futex_waitv with CLOCK_REALTIME */
>>> +	if (futex_get_abs_timeout(CLOCK_REALTIME, &to, timeout_ns))
>>> +		return RET_FAIL;
>>> +	res = futex_waitv(&waitv, 1, 0, &to, CLOCK_REALTIME);
>>> +	test_timeout(res, &ret, "futex_waitv realtime", ETIMEDOUT);
>>
>> Hi André,
>>
>> when built with -m32 and run as compat this two futex_waitv calls hang
>> on x86 and s390 (noticed while wiring up futex_waitv). The rest of the
>> futex selftests pass. This suggests some common compat issue? Any ideas?
> 
> The issue is that futex_waitv() only accepts struct timespec that uses
> 64bit members. When using -m32, glibc will give you a 32bit timespec,
> thus the timeout won't wort. Someday glibc will provide 64bit timespec
> to 32bit userspace, given that this is affected by y2038 bug.

We do since glibc 2.34, but you need to opt-in by defining -D_TIME_SIZE=64.
The default might change in a future release, so hopefully we will have
both LFS and 64-bit as the default ABI.

> 
> In previous submissions I added a workaround for that in the
> selftest[0]. Search for "Y2038 section for 32-bit applications" in that
> link. I'll submit something like that for futex_waitv() timeout test.
> 
> [0]
> https://lore.kernel.org/lkml/20210709001328.329716-6-andrealmeid@collabora.com/
>
Arnd Bergmann Nov. 9, 2021, 1:05 p.m. UTC | #4 
On Tue, Nov 9, 2021 at 1:52 PM André Almeida <andrealmeid@collabora.com> wrote:
> Às 08:18 de 09/11/21, Vasily Gorbik escreveu:
> > On Thu, Sep 23, 2021 at 02:11:09PM -0300, André Almeida wrote:
> >> Test if the futex_waitv timeout is working as expected, using the
> >> supported clockid options.
> >
> >> +    /* futex_waitv with CLOCK_MONOTONIC */
> >> +    if (futex_get_abs_timeout(CLOCK_MONOTONIC, &to, timeout_ns))
> >> +            return RET_FAIL;
> >> +    res = futex_waitv(&waitv, 1, 0, &to, CLOCK_MONOTONIC);
> >> +    test_timeout(res, &ret, "futex_waitv monotonic", ETIMEDOUT);
> >> +
> >> +    /* futex_waitv with CLOCK_REALTIME */
> >> +    if (futex_get_abs_timeout(CLOCK_REALTIME, &to, timeout_ns))
> >> +            return RET_FAIL;
> >> +    res = futex_waitv(&waitv, 1, 0, &to, CLOCK_REALTIME);
> >> +    test_timeout(res, &ret, "futex_waitv realtime", ETIMEDOUT);
> >
> > Hi André,
> >
> > when built with -m32 and run as compat this two futex_waitv calls hang
> > on x86 and s390 (noticed while wiring up futex_waitv). The rest of the
> > futex selftests pass. This suggests some common compat issue? Any ideas?
>
> The issue is that futex_waitv() only accepts struct timespec that uses
> 64bit members. When using -m32, glibc will give you a 32bit timespec,
> thus the timeout won't wort. Someday glibc will provide 64bit timespec
> to 32bit userspace, given that this is affected by y2038 bug.

I think in the latest glibc you should be able to pass -D_TIME_BITS=64 to
the compiler to get the correct definition. Unfortunately, this only works
for simple test cases, but breaks if you call any interfaces from another
(non-glibc) library that depend on a particular time_t definition.

Alistair Francis also recently posted a set of helpers for the old futex()
call to make that easier to use from applications regardless of the libc
interface. I think it would be good to have this for futex_waitv() as well.

       Arnd
diff mbox series

Patch

diff --git a/tools/testing/selftests/futex/functional/futex_wait_timeout.c b/tools/testing/selftests/futex/functional/futex_wait_timeout.c
index 1f8f6daaf1e7..3651ce17beeb 100644
--- a/tools/testing/selftests/futex/functional/futex_wait_timeout.c
+++ b/tools/testing/selftests/futex/functional/futex_wait_timeout.c
@@ -17,6 +17,7 @@ 
 
 #include <pthread.h>
 #include "futextest.h"
+#include "futex2test.h"
 #include "logging.h"
 
 #define TEST_NAME "futex-wait-timeout"
@@ -96,6 +97,12 @@  int main(int argc, char *argv[])
 	struct timespec to;
 	pthread_t thread;
 	int c;
+	struct futex_waitv waitv = {
+			.uaddr = (uintptr_t)&f1,
+			.val = f1,
+			.flags = FUTEX_32,
+			.__reserved = 0
+		};
 
 	while ((c = getopt(argc, argv, "cht:v:")) != -1) {
 		switch (c) {
@@ -118,7 +125,7 @@  int main(int argc, char *argv[])
 	}
 
 	ksft_print_header();
-	ksft_set_plan(7);
+	ksft_set_plan(9);
 	ksft_print_msg("%s: Block on a futex and wait for timeout\n",
 	       basename(argv[0]));
 	ksft_print_msg("\tArguments: timeout=%ldns\n", timeout_ns);
@@ -175,6 +182,18 @@  int main(int argc, char *argv[])
 	res = futex_lock_pi(&futex_pi, NULL, 0, FUTEX_CLOCK_REALTIME);
 	test_timeout(res, &ret, "futex_lock_pi invalid timeout flag", ENOSYS);
 
+	/* futex_waitv with CLOCK_MONOTONIC */
+	if (futex_get_abs_timeout(CLOCK_MONOTONIC, &to, timeout_ns))
+		return RET_FAIL;
+	res = futex_waitv(&waitv, 1, 0, &to, CLOCK_MONOTONIC);
+	test_timeout(res, &ret, "futex_waitv monotonic", ETIMEDOUT);
+
+	/* futex_waitv with CLOCK_REALTIME */
+	if (futex_get_abs_timeout(CLOCK_REALTIME, &to, timeout_ns))
+		return RET_FAIL;
+	res = futex_waitv(&waitv, 1, 0, &to, CLOCK_REALTIME);
+	test_timeout(res, &ret, "futex_waitv realtime", ETIMEDOUT);
+
 	ksft_print_cnts();
 	return ret;
 }