All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH-for-5.1?] tests/tcg/multiarch/linux-test: Skip test if nanosleep missing (Travis)
@ 2020-07-21  8:57 Philippe Mathieu-Daudé
  2020-07-21 11:38 ` Laurent Vivier
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-21  8:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Alex Bennée, Laurent Vivier,
	Philippe Mathieu-Daudé

The time test sometimes fails on Travis-CI [*]:

    TEST    linux-test on aarch64
  tests/tcg/multiarch/linux-test.c:237: nanosleep
  make[2]: *** [run-linux-test] Error 1
  make: *** [run-tcg-tests-aarch64-linux-user] Error 2

As this seems due to a container limitation on Travis-CI,
simply skip the test there.

[*] https://travis-ci.org/github/qemu/qemu/jobs/710005078#L3706

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC because per Laurent Vivier we are not using the correct libc
    while cross-linking the test (maybe change in the container
    packages?)
---
 tests/tcg/multiarch/linux-test.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c
index 8a7c15cd31..c7dfdec9ec 100644
--- a/tests/tcg/multiarch/linux-test.c
+++ b/tests/tcg/multiarch/linux-test.c
@@ -233,8 +233,13 @@ static void test_time(void)
     ts.tv_sec = 0;
     ts.tv_nsec = 20 * 1000000;
     chk_error(nanosleep(&ts, &rem));
-    if (rem.tv_sec != 1)
+    if (rem.tv_sec != 1) {
+        if (getenv("TRAVIS_ARCH")) {
+            printf("nanosleep missing? skipping 'time' test\n");
+            return;
+        }
         error("nanosleep");
+    }
     chk_error(gettimeofday(&tv2, NULL));
     ti = tv2.tv_sec - tv.tv_sec;
     if (ti >= 2)
-- 
2.21.3



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH-for-5.1?] tests/tcg/multiarch/linux-test: Skip test if nanosleep missing (Travis)
  2020-07-21  8:57 [RFC PATCH-for-5.1?] tests/tcg/multiarch/linux-test: Skip test if nanosleep missing (Travis) Philippe Mathieu-Daudé
@ 2020-07-21 11:38 ` Laurent Vivier
  2020-07-21 12:24   ` Laurent Vivier
  2020-07-21 12:32   ` Alex Bennée
  0 siblings, 2 replies; 5+ messages in thread
From: Laurent Vivier @ 2020-07-21 11:38 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Thomas Huth, Alex Bennée

Le 21/07/2020 à 10:57, Philippe Mathieu-Daudé a écrit :
> The time test sometimes fails on Travis-CI [*]:
> 
>     TEST    linux-test on aarch64
>   tests/tcg/multiarch/linux-test.c:237: nanosleep
>   make[2]: *** [run-linux-test] Error 1
>   make: *** [run-tcg-tests-aarch64-linux-user] Error 2
> 
> As this seems due to a container limitation on Travis-CI,
> simply skip the test there.
> 
> [*] https://travis-ci.org/github/qemu/qemu/jobs/710005078#L3706
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> RFC because per Laurent Vivier we are not using the correct libc
>     while cross-linking the test (maybe change in the container
>     packages?)
> ---
>  tests/tcg/multiarch/linux-test.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c
> index 8a7c15cd31..c7dfdec9ec 100644
> --- a/tests/tcg/multiarch/linux-test.c
> +++ b/tests/tcg/multiarch/linux-test.c
> @@ -233,8 +233,13 @@ static void test_time(void)
>      ts.tv_sec = 0;
>      ts.tv_nsec = 20 * 1000000;
>      chk_error(nanosleep(&ts, &rem));
> -    if (rem.tv_sec != 1)
> +    if (rem.tv_sec != 1) {
> +        if (getenv("TRAVIS_ARCH")) {
> +            printf("nanosleep missing? skipping 'time' test\n");
> +            return;
> +        }
>          error("nanosleep");
> +    }
>      chk_error(gettimeofday(&tv2, NULL));
>      ti = tv2.tv_sec - tv.tv_sec;
>      if (ti >= 2)
> 

Well, in the end I think the problem is in linux-user:

We copy the "rem" structure even if there is no error, so "1" is
overwritten.

We don't have the problem on all architectures because some use
nanosleep() syscall (that is correct) others use clock_nanosleep()
syscall that is not correct.

This should fix the problem:

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1211e759c26c..130005716ece 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11831,7 +11831,7 @@ static abi_long do_syscall1(void *cpu_env, int
num, abi_long arg1,
         target_to_host_timespec(&ts, arg3);
         ret = get_errno(safe_clock_nanosleep(arg1, arg2,
                                              &ts, arg4 ? &ts : NULL));
-        if (arg4)
+        if (is_error(ret) && arg4)
             host_to_target_timespec(arg4, &ts);

 #if defined(TARGET_PPC)

Thanks,
Laurent



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH-for-5.1?] tests/tcg/multiarch/linux-test: Skip test if nanosleep missing (Travis)
  2020-07-21 11:38 ` Laurent Vivier
@ 2020-07-21 12:24   ` Laurent Vivier
  2020-07-21 15:26     ` Philippe Mathieu-Daudé
  2020-07-21 12:32   ` Alex Bennée
  1 sibling, 1 reply; 5+ messages in thread
From: Laurent Vivier @ 2020-07-21 12:24 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Thomas Huth, Alex Bennée

Le 21/07/2020 à 13:38, Laurent Vivier a écrit :
> Le 21/07/2020 à 10:57, Philippe Mathieu-Daudé a écrit :
>> The time test sometimes fails on Travis-CI [*]:
>>
>>     TEST    linux-test on aarch64
>>   tests/tcg/multiarch/linux-test.c:237: nanosleep
>>   make[2]: *** [run-linux-test] Error 1
>>   make: *** [run-tcg-tests-aarch64-linux-user] Error 2
>>
>> As this seems due to a container limitation on Travis-CI,
>> simply skip the test there.
>>
>> [*] https://travis-ci.org/github/qemu/qemu/jobs/710005078#L3706
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> RFC because per Laurent Vivier we are not using the correct libc
>>     while cross-linking the test (maybe change in the container
>>     packages?)
>> ---
>>  tests/tcg/multiarch/linux-test.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c
>> index 8a7c15cd31..c7dfdec9ec 100644
>> --- a/tests/tcg/multiarch/linux-test.c
>> +++ b/tests/tcg/multiarch/linux-test.c
>> @@ -233,8 +233,13 @@ static void test_time(void)
>>      ts.tv_sec = 0;
>>      ts.tv_nsec = 20 * 1000000;
>>      chk_error(nanosleep(&ts, &rem));
>> -    if (rem.tv_sec != 1)
>> +    if (rem.tv_sec != 1) {
>> +        if (getenv("TRAVIS_ARCH")) {
>> +            printf("nanosleep missing? skipping 'time' test\n");
>> +            return;
>> +        }
>>          error("nanosleep");
>> +    }
>>      chk_error(gettimeofday(&tv2, NULL));
>>      ti = tv2.tv_sec - tv.tv_sec;
>>      if (ti >= 2)
>>
> 
> Well, in the end I think the problem is in linux-user:
> 
> We copy the "rem" structure even if there is no error, so "1" is
> overwritten.
> 
> We don't have the problem on all architectures because some use
> nanosleep() syscall (that is correct) others use clock_nanosleep()
> syscall that is not correct.
> 
> This should fix the problem:
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c26c..130005716ece 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11831,7 +11831,7 @@ static abi_long do_syscall1(void *cpu_env, int
> num, abi_long arg1,
>          target_to_host_timespec(&ts, arg3);
>          ret = get_errno(safe_clock_nanosleep(arg1, arg2,
>                                               &ts, arg4 ? &ts : NULL));
> -        if (arg4)
> +        if (is_error(ret) && arg4)
>              host_to_target_timespec(arg4, &ts);
> 
>  #if defined(TARGET_PPC)

According to clock_nanosleep(2) it should be in fact:

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1211e759c26c..63e7cd8947e5 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11831,8 +11831,9 @@ static abi_long do_syscall1(void *cpu_env, int
num, abi_long arg1,
         target_to_host_timespec(&ts, arg3);
         ret = get_errno(safe_clock_nanosleep(arg1, arg2,
                                              &ts, arg4 ? &ts : NULL));
-        if (arg4)
+        if (ret == -TARGET_EINTR && arg4 && arg2 != TIMER_ABSTIME) {
             host_to_target_timespec(arg4, &ts);
+        }

 #if defined(TARGET_PPC)
         /* clock_nanosleep is odd in that it returns positive errno values.



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH-for-5.1?] tests/tcg/multiarch/linux-test: Skip test if nanosleep missing (Travis)
  2020-07-21 11:38 ` Laurent Vivier
  2020-07-21 12:24   ` Laurent Vivier
@ 2020-07-21 12:32   ` Alex Bennée
  1 sibling, 0 replies; 5+ messages in thread
From: Alex Bennée @ 2020-07-21 12:32 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Thomas Huth, Philippe Mathieu-Daudé, qemu-devel


Laurent Vivier <laurent@vivier.eu> writes:

> Le 21/07/2020 à 10:57, Philippe Mathieu-Daudé a écrit :
>> The time test sometimes fails on Travis-CI [*]:
>> 
>>     TEST    linux-test on aarch64
>>   tests/tcg/multiarch/linux-test.c:237: nanosleep
>>   make[2]: *** [run-linux-test] Error 1
>>   make: *** [run-tcg-tests-aarch64-linux-user] Error 2
>> 
>> As this seems due to a container limitation on Travis-CI,
>> simply skip the test there.
>> 
>> [*] https://travis-ci.org/github/qemu/qemu/jobs/710005078#L3706
>> 
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> RFC because per Laurent Vivier we are not using the correct libc
>>     while cross-linking the test (maybe change in the container
>>     packages?)
>> ---
>>  tests/tcg/multiarch/linux-test.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c
>> index 8a7c15cd31..c7dfdec9ec 100644
>> --- a/tests/tcg/multiarch/linux-test.c
>> +++ b/tests/tcg/multiarch/linux-test.c
>> @@ -233,8 +233,13 @@ static void test_time(void)
>>      ts.tv_sec = 0;
>>      ts.tv_nsec = 20 * 1000000;
>>      chk_error(nanosleep(&ts, &rem));
>> -    if (rem.tv_sec != 1)
>> +    if (rem.tv_sec != 1) {
>> +        if (getenv("TRAVIS_ARCH")) {
>> +            printf("nanosleep missing? skipping 'time' test\n");
>> +            return;
>> +        }
>>          error("nanosleep");
>> +    }
>>      chk_error(gettimeofday(&tv2, NULL));
>>      ti = tv2.tv_sec - tv.tv_sec;
>>      if (ti >= 2)
>> 
>
> Well, in the end I think the problem is in linux-user:
>
> We copy the "rem" structure even if there is no error, so "1" is
> overwritten.
>
> We don't have the problem on all architectures because some use
> nanosleep() syscall (that is correct) others use clock_nanosleep()
> syscall that is not correct.
>
> This should fix the problem:
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c26c..130005716ece 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11831,7 +11831,7 @@ static abi_long do_syscall1(void *cpu_env, int
> num, abi_long arg1,
>          target_to_host_timespec(&ts, arg3);
>          ret = get_errno(safe_clock_nanosleep(arg1, arg2,
>                                               &ts, arg4 ? &ts : NULL));
> -        if (arg4)
> +        if (is_error(ret) && arg4)
>              host_to_target_timespec(arg4, &ts);

So the testcase worked and caught something :-)

I don't know why I couldn't trigger it reliably on my own dev setting though...

>
>  #if defined(TARGET_PPC)
>
> Thanks,
> Laurent


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH-for-5.1?] tests/tcg/multiarch/linux-test: Skip test if nanosleep missing (Travis)
  2020-07-21 12:24   ` Laurent Vivier
@ 2020-07-21 15:26     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-21 15:26 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel; +Cc: Thomas Huth, Alex Bennée

On 7/21/20 2:24 PM, Laurent Vivier wrote:
> Le 21/07/2020 à 13:38, Laurent Vivier a écrit :
>> Le 21/07/2020 à 10:57, Philippe Mathieu-Daudé a écrit :
>>> The time test sometimes fails on Travis-CI [*]:
>>>
>>>     TEST    linux-test on aarch64
>>>   tests/tcg/multiarch/linux-test.c:237: nanosleep
>>>   make[2]: *** [run-linux-test] Error 1
>>>   make: *** [run-tcg-tests-aarch64-linux-user] Error 2
>>>
>>> As this seems due to a container limitation on Travis-CI,
>>> simply skip the test there.
>>>
>>> [*] https://travis-ci.org/github/qemu/qemu/jobs/710005078#L3706
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> RFC because per Laurent Vivier we are not using the correct libc
>>>     while cross-linking the test (maybe change in the container
>>>     packages?)
>>> ---
>>>  tests/tcg/multiarch/linux-test.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/tcg/multiarch/linux-test.c b/tests/tcg/multiarch/linux-test.c
>>> index 8a7c15cd31..c7dfdec9ec 100644
>>> --- a/tests/tcg/multiarch/linux-test.c
>>> +++ b/tests/tcg/multiarch/linux-test.c
>>> @@ -233,8 +233,13 @@ static void test_time(void)
>>>      ts.tv_sec = 0;
>>>      ts.tv_nsec = 20 * 1000000;
>>>      chk_error(nanosleep(&ts, &rem));
>>> -    if (rem.tv_sec != 1)
>>> +    if (rem.tv_sec != 1) {
>>> +        if (getenv("TRAVIS_ARCH")) {
>>> +            printf("nanosleep missing? skipping 'time' test\n");
>>> +            return;
>>> +        }
>>>          error("nanosleep");
>>> +    }
>>>      chk_error(gettimeofday(&tv2, NULL));
>>>      ti = tv2.tv_sec - tv.tv_sec;
>>>      if (ti >= 2)
>>>
>>
>> Well, in the end I think the problem is in linux-user:
>>
>> We copy the "rem" structure even if there is no error, so "1" is
>> overwritten.
>>
>> We don't have the problem on all architectures because some use
>> nanosleep() syscall (that is correct) others use clock_nanosleep()
>> syscall that is not correct.
>>
>> This should fix the problem:
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 1211e759c26c..130005716ece 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -11831,7 +11831,7 @@ static abi_long do_syscall1(void *cpu_env, int
>> num, abi_long arg1,
>>          target_to_host_timespec(&ts, arg3);
>>          ret = get_errno(safe_clock_nanosleep(arg1, arg2,
>>                                               &ts, arg4 ? &ts : NULL));
>> -        if (arg4)
>> +        if (is_error(ret) && arg4)
>>              host_to_target_timespec(arg4, &ts);
>>
>>  #if defined(TARGET_PPC)
> 
> According to clock_nanosleep(2) it should be in fact:
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c26c..63e7cd8947e5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -11831,8 +11831,9 @@ static abi_long do_syscall1(void *cpu_env, int
> num, abi_long arg1,
>          target_to_host_timespec(&ts, arg3);
>          ret = get_errno(safe_clock_nanosleep(arg1, arg2,
>                                               &ts, arg4 ? &ts : NULL));
> -        if (arg4)
> +        if (ret == -TARGET_EINTR && arg4 && arg2 != TIMER_ABSTIME) {
>              host_to_target_timespec(arg4, &ts);
> +        }

I tested this hunk and couldn't reproduce it on Travis-CI. Since it is
intermittent, that doesn't mean much, but still if you send a proper
path you can add "Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>".

> 
>  #if defined(TARGET_PPC)
>          /* clock_nanosleep is odd in that it returns positive errno values.
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-21 15:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-21  8:57 [RFC PATCH-for-5.1?] tests/tcg/multiarch/linux-test: Skip test if nanosleep missing (Travis) Philippe Mathieu-Daudé
2020-07-21 11:38 ` Laurent Vivier
2020-07-21 12:24   ` Laurent Vivier
2020-07-21 15:26     ` Philippe Mathieu-Daudé
2020-07-21 12:32   ` Alex Bennée

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.