[PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[Yu Liao]

When building selftests/vDSO:
$ make -C tools/testing/selftests TARGETS=vDSO
I hit the following compilation error:

  vdso_test_getrandom.c:260:17: error: 'CLONE_NEWTIME' undeclared
  (first use in this function); did you mean 'CLONE_NEWIPC'?
    260 |  assert(unshare(CLONE_NEWTIME) == 0);
        |                 ^~~~~~~~~~~~~

CLONE_NEWTIME is defined in linux/sched.h, so fix this by including
<linux/sched.h>.

Fixes: 2aec90036dcd ("selftests: vDSO: ensure vgetrandom works in a time namespace")
Signed-off-by: Yu Liao <liaoyu15@huawei.com>
---
 tools/testing/selftests/vDSO/vdso_test_getrandom.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
index 72a1d9b43a84..84f2bbb2d5e0 100644
--- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
+++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
@@ -19,6 +19,7 @@
 #include <sys/ptrace.h>
 #include <sys/wait.h>
 #include <sys/types.h>
+#include <linux/sched.h>
 #include <linux/random.h>
 #include <linux/compiler.h>
 #include <linux/ptrace.h>
-- 
2.33.0

Re: [PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[Shuah Khan]

On 9/19/24 05:18, Yu Liao wrote:
> When building selftests/vDSO:
> $ make -C tools/testing/selftests TARGETS=vDSO
> I hit the following compilation error:
> 
>    vdso_test_getrandom.c:260:17: error: 'CLONE_NEWTIME' undeclared
>    (first use in this function); did you mean 'CLONE_NEWIPC'?
>      260 |  assert(unshare(CLONE_NEWTIME) == 0);
>          |                 ^~~~~~~~~~~~~
> 
> CLONE_NEWTIME is defined in linux/sched.h, so fix this by including
> <linux/sched.h>.
> 
> Fixes: 2aec90036dcd ("selftests: vDSO: ensure vgetrandom works in a time namespace")
> Signed-off-by: Yu Liao <liaoyu15@huawei.com>
> ---
>   tools/testing/selftests/vDSO/vdso_test_getrandom.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
> index 72a1d9b43a84..84f2bbb2d5e0 100644
> --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
> +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
> @@ -19,6 +19,7 @@
>   #include <sys/ptrace.h>
>   #include <sys/wait.h>
>   #include <sys/types.h>
> +#include <linux/sched.h>
>   #include <linux/random.h>
>   #include <linux/compiler.h>
>   #include <linux/ptrace.h>

Do you see this error after installing headers? Installing headers is
a dependency to be able to compile selftests.

thanks,
-- Shuah

Re: [PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[Yu Liao]

On 2024/9/20 0:51, Shuah Khan wrote:

>> diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
>> b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
>> index 72a1d9b43a84..84f2bbb2d5e0 100644
>> --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
>> +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
>> @@ -19,6 +19,7 @@
>>   #include <sys/ptrace.h>
>>   #include <sys/wait.h>
>>   #include <sys/types.h>
>> +#include <linux/sched.h>
>>   #include <linux/random.h>
>>   #include <linux/compiler.h>
>>   #include <linux/ptrace.h>
> 
> Do you see this error after installing headers? Installing headers is
> a dependency to be able to compile selftests.
> 

Yes, this error still exists after installing header files. Here are my steps
to reproduce:

make headers_install
make -C tools/testing/selftests TARGETS=vDSO

After applying the patch, the error no longer appears.

Best regards,
Yu

Re: [PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[Christophe Leroy]

Le 20/09/2024 à 03:54, Yu Liao a écrit :
> [Vous ne recevez pas souvent de courriers de liaoyu15@huawei.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> On 2024/9/20 0:51, Shuah Khan wrote:
> 
>>> diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
>>> b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
>>> index 72a1d9b43a84..84f2bbb2d5e0 100644
>>> --- a/tools/testing/selftests/vDSO/vdso_test_getrandom.c
>>> +++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
>>> @@ -19,6 +19,7 @@
>>>    #include <sys/ptrace.h>
>>>    #include <sys/wait.h>
>>>    #include <sys/types.h>
>>> +#include <linux/sched.h>

This not correct. According to man page of clone(2), CLONE_ macros are 
in <sched.h>, you should include <sched.h> instead.

See https://man7.org/linux/man-pages/man2/clone.2.html

By the way on my environment (gcc 12.2 + glibc 2.38), <sched.h> gets 
already indirectly included by <phread.h>, and at the end the 
CLONE_NEWTIME is in glibc headers in usr/include/bits/sched.h

>>>    #include <linux/random.h>
>>>    #include <linux/compiler.h>
>>>    #include <linux/ptrace.h>
>>
>> Do you see this error after installing headers? Installing headers is
>> a dependency to be able to compile selftests.
>>
> 
> Yes, this error still exists after installing header files. Here are my steps
> to reproduce:
> 
> make headers_install
> make -C tools/testing/selftests TARGETS=vDSO
> 
> After applying the patch, the error no longer appears.
> 
> Best regards,
> Yu

Re: [PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[Jason A. Donenfeld]

Indeed probably <sched.h> is what's wanted here.

Shuah - I was taking patches for this code during the 6.12 development
cycle, because it all interacted with the dev work I was doing. But I
think that's died down now and things can return to normal. Do you
want to take these like you usually do now? Or is that annoying
because your tree won't be based on them for another cycle, so I
should still do it? I'm okay with doing whatever you want.

Jason

Re: [PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[Shuah Khan]

On 9/20/24 09:31, Jason A. Donenfeld wrote:
> Indeed probably <sched.h> is what's wanted here.
> 

Yu Liao, Please send v2 as per the review comments.

> Shuah - I was taking patches for this code during the 6.12 development
> cycle, because it all interacted with the dev work I was doing. But I
> think that's died down now and things can return to normal. Do you
> want to take these like you usually do now? Or is that annoying
> because your tree won't be based on them for another cycle, so I
> should still do it? I'm okay with doing whatever you want.
> 
> Jason

I can take this through my tree - I rebase as soon as rc1 comes out.
We can also handshake as needed if there are conflicts still.

thanks,
-- Shuah

Re: [PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[Yu Liao]

Hi,
On 2024/9/23 23:30, Shuah Khan wrote:
> On 9/20/24 09:31, Jason A. Donenfeld wrote:
>> Indeed probably <sched.h> is what's wanted here.
>>
> 
> Yu Liao, Please send v2 as per the review comments.
> 

CLONE_NEWTIME was introduced in glibc-2.36, which was released in August 2022.
As Christophe mentioned, <sched.h> is already indirectly included by
<phread.h>, so this issue does not exist if glibc version higher than 2.36.

Additionally, CLONE_NEWTIME was introduced in Linux 5.6 in March 2020, the
CLONE_ macros are also defined in <linux/sched.h>, which is part of the
kernel-header package.

My environment is Ubuntu 22.04 (Linux 5.15 + glibc 2.35), after upgrading to a
newer version of glibc, the issue appears to be resolved.
It seems to me that including <sched.h> might be unnecessary. I would greatly
appreciate your guidance on how best to handle this situation.

Best regards,
Yu

Re: [PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[SurajSonawane2415]

Yes, I ran it but faced an issue. After reading comments on a similar patch shared by Jason A. 
Donenfeld, I realized that my environment is Ubuntu 22.04 (Linux 5.15 + glibc 2.35). 
After upgrading to a newer version of glibc, the issue seems to be fixed. 
I think including <sched.h> might not be needed. I would really appreciate your advice on how to handle this.

Re: [PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[Shuah Khan]

On 9/23/24 20:57, Yu Liao wrote:
> Hi,
> On 2024/9/23 23:30, Shuah Khan wrote:
>> On 9/20/24 09:31, Jason A. Donenfeld wrote:
>>> Indeed probably <sched.h> is what's wanted here.
>>>
>>
>> Yu Liao, Please send v2 as per the review comments.
>>
> 
> CLONE_NEWTIME was introduced in glibc-2.36, which was released in August 2022.
> As Christophe mentioned, <sched.h> is already indirectly included by
> <phread.h>, so this issue does not exist if glibc version higher than 2.36.
> 
> Additionally, CLONE_NEWTIME was introduced in Linux 5.6 in March 2020, the
> CLONE_ macros are also defined in <linux/sched.h>, which is part of the
> kernel-header package.
> 
> My environment is Ubuntu 22.04 (Linux 5.15 + glibc 2.35), after upgrading to a
> newer version of glibc, the issue appears to be resolved.
> It seems to me that including <sched.h> might be unnecessary. I would greatly
> appreciate your guidance on how best to handle this situation.
> 

Please send me v2 with sched.h included explicitly. It is better do do so
than having it come from <phread.h> indirectly.

thanks,
-- SHuah

Re: [PATCH] selftests: vDSO: fix compile error for vdso_test_getrandom

[Yu Liao]

On 2024/10/4 6:21, Shuah Khan wrote:
> Please send me v2 with sched.h included explicitly. It is better do do so
> than having it come from <phread.h> indirectly.
> 

Sorry for the late response, I have sent the v2 patch.

https://lore.kernel.org/all/20241008023332.19902-1-liaoyu15@huawei.com/

Best regards,
Yu