[PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Muhammad Usama Anjum]

The value of __NR_userfaultfd was changed to 282 when
asm-generic/unistd.h was included. It makes the test to fail every time
as the correct number of this syscall on x86_64 is 323. Fix the header
to asm/unistd.h.

Fixes: a5c6bc590094 ("selftests/mm: remove local __NR_* definitions")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/mm/pagemap_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
index fc90af2a97b80..bcc73b4e805c6 100644
--- a/tools/testing/selftests/mm/pagemap_ioctl.c
+++ b/tools/testing/selftests/mm/pagemap_ioctl.c
@@ -15,7 +15,7 @@
 #include <sys/ioctl.h>
 #include <sys/stat.h>
 #include <math.h>
-#include <asm-generic/unistd.h>
+#include <asm/unistd.h>
 #include <pthread.h>
 #include <sys/resource.h>
 #include <assert.h>
-- 
2.39.2

[PATCH 2/2] kselftests: mm: Fail the test if userfaultfd syscall isn't found

[Muhammad Usama Anjum]

The userfaultfd is enabled in the config fragment of mm selftest suite.
It must always be present. If it isn't present, we should throw error
and not just skip. This would have helped us catch the test breakage.
Adding this now to catch the future breakages.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/mm/pagemap_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
index bcc73b4e805c6..d83dda8edf62c 100644
--- a/tools/testing/selftests/mm/pagemap_ioctl.c
+++ b/tools/testing/selftests/mm/pagemap_ioctl.c
@@ -95,7 +95,7 @@ int init_uffd(void)
 
 	uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY);
 	if (uffd == -1)
-		return uffd;
+		ksft_exit_fail_perror("Userfaultfd syscall failed");
 
 	uffdio_api.api = UFFD_API;
 	uffdio_api.features = UFFD_FEATURE_WP_UNPOPULATED | UFFD_FEATURE_WP_ASYNC |
-- 
2.39.2

Re: [PATCH 2/2] kselftests: mm: Fail the test if userfaultfd syscall isn't found

[Shuah Khan]

On 9/12/24 04:31, Muhammad Usama Anjum wrote:
> The userfaultfd is enabled in the config fragment of mm selftest suite.
> It must always be present. If it isn't present, we should throw error
> and not just skip. This would have helped us catch the test breakage.

Please elaborate on this to help understand the what breakage was
missed.

Also this commit log doesn't look right to me. syscall() could
fail for any reason. Do you mean to see skip is incorrect in this
error leg? Please see comments below.

> Adding this now to catch the future breakages.
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>   tools/testing/selftests/mm/pagemap_ioctl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
> index bcc73b4e805c6..d83dda8edf62c 100644
> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
> @@ -95,7 +95,7 @@ int init_uffd(void)
>   
>   	uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY);
>   	if (uffd == -1)
> -		return uffd;
> +		ksft_exit_fail_perror("Userfaultfd syscall failed");

This looks wrong to me - Is missing config the only reason this syscall
would fail?

>   
>   	uffdio_api.api = UFFD_API;
>   	uffdio_api.features = UFFD_FEATURE_WP_UNPOPULATED | UFFD_FEATURE_WP_ASYNC |

thanks,
-- Shuah

Re: [PATCH 2/2] kselftests: mm: Fail the test if userfaultfd syscall isn't found

[Shuah Khan]

On 9/12/24 10:10, Shuah Khan wrote:
> On 9/12/24 04:31, Muhammad Usama Anjum wrote:
>> The userfaultfd is enabled in the config fragment of mm selftest suite.
>> It must always be present. If it isn't present, we should throw error
>> and not just skip. This would have helped us catch the test breakage.
> 
> Please elaborate on this to help understand the what breakage was
> missed.
> 
> Also this commit log doesn't look right to me. syscall() could
> fail for any reason. Do you mean to see skip is incorrect in this
> error leg? Please see comments below.
> 
>> Adding this now to catch the future breakages.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>   tools/testing/selftests/mm/pagemap_ioctl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
>> index bcc73b4e805c6..d83dda8edf62c 100644
>> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
>> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
>> @@ -95,7 +95,7 @@ int init_uffd(void)
>>       uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY);
>>       if (uffd == -1)
>> -        return uffd;
>> +        ksft_exit_fail_perror("Userfaultfd syscall failed");
> 
> This looks wrong to me - Is missing config the only reason this syscall
> would fail?

It should still skip if __NR_userfaultfd isn't supported on a release
or an architecture.

The real problem seems to be in main():

if (init_uffd())
                 ksft_exit_pass();


Why is this ksft_exit_pass()? Looks like further investigation is
necessary to understand the problem and fix.

thanks,
-- Shuah

Re: [PATCH 2/2] kselftests: mm: Fail the test if userfaultfd syscall isn't found

[Muhammad Usama Anjum]

On 9/12/24 10:28 PM, Shuah Khan wrote:
> On 9/12/24 10:10, Shuah Khan wrote:
>> On 9/12/24 04:31, Muhammad Usama Anjum wrote:
>>> The userfaultfd is enabled in the config fragment of mm selftest suite.
>>> It must always be present. If it isn't present, we should throw error
>>> and not just skip. This would have helped us catch the test breakage.
>>
>> Please elaborate on this to help understand the what breakage was
>> missed.
>>
>> Also this commit log doesn't look right to me. syscall() could
>> fail for any reason. Do you mean to see skip is incorrect in this
>> error leg? Please see comments below.
>>
>>> Adding this now to catch the future breakages.
>>>
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>>>   tools/testing/selftests/mm/pagemap_ioctl.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c
>>> b/tools/testing/selftests/mm/pagemap_ioctl.c
>>> index bcc73b4e805c6..d83dda8edf62c 100644
>>> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
>>> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
>>> @@ -95,7 +95,7 @@ int init_uffd(void)
>>>       uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK |
>>> UFFD_USER_MODE_ONLY);
>>>       if (uffd == -1)
>>> -        return uffd;
>>> +        ksft_exit_fail_perror("Userfaultfd syscall failed");
>>
>> This looks wrong to me - Is missing config the only reason this syscall
>> would fail?
> 
> It should still skip if __NR_userfaultfd isn't supported on a release
> or an architecture.
> 
> The real problem seems to be in main():
> 
> if (init_uffd())
>                 ksft_exit_pass();
> 
> 
> Why is this ksft_exit_pass()? Looks like further investigation is
> necessary to understand the problem and fix.
Let's skip this patch as it'll create more noise on unsupported
architectures than catching failures on supported architectures.

> 
> thanks,
> -- Shuah

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Shuah Khan]

On 9/12/24 04:31, Muhammad Usama Anjum wrote:
> The value of __NR_userfaultfd was changed to 282 when
> asm-generic/unistd.h was included. It makes the test to fail every time
> as the correct number of this syscall on x86_64 is 323. Fix the header
> to asm/unistd.h.
> 

"please elaborate every time" - I just built on my x86_64 and built
just fine. I am not saying this isn't a problem, it is good to
understand why and how it is failing before making the change.

> Fixes: a5c6bc590094 ("selftests/mm: remove local __NR_* definitions")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>   tools/testing/selftests/mm/pagemap_ioctl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c
> index fc90af2a97b80..bcc73b4e805c6 100644
> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
> @@ -15,7 +15,7 @@
>   #include <sys/ioctl.h>
>   #include <sys/stat.h>
>   #include <math.h>
> -#include <asm-generic/unistd.h>
> +#include <asm/unistd.h>
>   #include <pthread.h>
>   #include <sys/resource.h>
>   #include <assert.h>

Also please generate a series with these two patches with cover-letter.

thanks,
-- Shuah

Re: [PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Muhammad Usama Anjum]

On 9/12/24 8:44 PM, Shuah Khan wrote:
> On 9/12/24 04:31, Muhammad Usama Anjum wrote:
>> The value of __NR_userfaultfd was changed to 282 when
>> asm-generic/unistd.h was included. It makes the test to fail every time
>> as the correct number of this syscall on x86_64 is 323. Fix the header
>> to asm/unistd.h.
>>
> 
> "please elaborate every time" - I just built on my x86_64 and built
> just fine.
The build isn't broken.

> I am not saying this isn't a problem, it is good to
> understand why and how it is failing before making the change.
I mean to say that the test is failing at run time because the correct
userfaultfd syscall isn't being found with __NR_userfaultfd = 282.
_NR_userfaultfd's value depends on the header. When asm-generic/unistd.h
is included, its value (282) is wrong. I've tested on x86_64.

The fix is simple. Add the correct header which has _NR_userfaultfd = 323.

> 
>> Fixes: a5c6bc590094 ("selftests/mm: remove local __NR_* definitions")
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>   tools/testing/selftests/mm/pagemap_ioctl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c
>> b/tools/testing/selftests/mm/pagemap_ioctl.c
>> index fc90af2a97b80..bcc73b4e805c6 100644
>> --- a/tools/testing/selftests/mm/pagemap_ioctl.c
>> +++ b/tools/testing/selftests/mm/pagemap_ioctl.c
>> @@ -15,7 +15,7 @@
>>   #include <sys/ioctl.h>
>>   #include <sys/stat.h>
>>   #include <math.h>
>> -#include <asm-generic/unistd.h>
>> +#include <asm/unistd.h>
>>   #include <pthread.h>
>>   #include <sys/resource.h>
>>   #include <assert.h>
> 
> Also please generate a series with these two patches with cover-letter.
> 
> thanks,
> -- Shuah

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Shuah Khan]

On 9/16/24 00:32, Muhammad Usama Anjum wrote:
> On 9/12/24 8:44 PM, Shuah Khan wrote:
>> On 9/12/24 04:31, Muhammad Usama Anjum wrote:
>>> The value of __NR_userfaultfd was changed to 282 when
>>> asm-generic/unistd.h was included. It makes the test to fail every time
>>> as the correct number of this syscall on x86_64 is 323. Fix the header
>>> to asm/unistd.h.
>>>
>>
>> "please elaborate every time" - I just built on my x86_64 and built
>> just fine.
> The build isn't broken.
> 
>> I am not saying this isn't a problem, it is good to
>> understand why and how it is failing before making the change.
> I mean to say that the test is failing at run time because the correct
> userfaultfd syscall isn't being found with __NR_userfaultfd = 282.
> _NR_userfaultfd's value depends on the header. When asm-generic/unistd.h
> is included, its value (282) is wrong. I've tested on x86_64.
> 

Okay - how do you know this is wrong? can you provide more details.

git grep _NR_userfaultfd
include/uapi/asm-generic/unistd.h:#define __NR_userfaultfd 282
include/uapi/asm-generic/unistd.h:__SYSCALL(__NR_userfaultfd, sys_userfaultfd)
tools/include/uapi/asm-generic/unistd.h:#define __NR_userfaultfd 282

> The fix is simple. Add the correct header which has _NR_userfaultfd = 323.

I need more details on this number.

thanks,
-- Shuah

Re: [PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Muhammad Usama Anjum]

On 9/17/24 6:56 AM, Shuah Khan wrote:
> On 9/16/24 00:32, Muhammad Usama Anjum wrote:
>> On 9/12/24 8:44 PM, Shuah Khan wrote:
>>> On 9/12/24 04:31, Muhammad Usama Anjum wrote:
>>>> The value of __NR_userfaultfd was changed to 282 when
>>>> asm-generic/unistd.h was included. It makes the test to fail every time
>>>> as the correct number of this syscall on x86_64 is 323. Fix the header
>>>> to asm/unistd.h.
>>>>
>>>
>>> "please elaborate every time" - I just built on my x86_64 and built
>>> just fine.
>> The build isn't broken.
>>
>>> I am not saying this isn't a problem, it is good to
>>> understand why and how it is failing before making the change.
>> I mean to say that the test is failing at run time because the correct
>> userfaultfd syscall isn't being found with __NR_userfaultfd = 282.
>> _NR_userfaultfd's value depends on the header. When asm-generic/unistd.h
>> is included, its value (282) is wrong. I've tested on x86_64.
>>
> 
> Okay - how do you know this is wrong? can you provide more details.
> 
> git grep _NR_userfaultfd
> include/uapi/asm-generic/unistd.h:#define __NR_userfaultfd 282
> include/uapi/asm-generic/unistd.h:__SYSCALL(__NR_userfaultfd,
> sys_userfaultfd)
> tools/include/uapi/asm-generic/unistd.h:#define __NR_userfaultfd 282
> 
>> The fix is simple. Add the correct header which has _NR_userfaultfd =
>> 323.

grep -rnIF "#define __NR_userfaultfd"
tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
arch/x86/include/generated/uapi/asm/unistd_32.h:374:#define
__NR_userfaultfd 374
arch/x86/include/generated/uapi/asm/unistd_64.h:327:#define
__NR_userfaultfd 323
arch/x86/include/generated/uapi/asm/unistd_x32.h:282:#define
__NR_userfaultfd (__X32_SYSCALL_BIT + 323)
arch/arm/include/generated/uapi/asm/unistd-eabi.h:347:#define
__NR_userfaultfd (__NR_SYSCALL_BASE + 388)
arch/arm/include/generated/uapi/asm/unistd-oabi.h:359:#define
__NR_userfaultfd (__NR_SYSCALL_BASE + 388)
include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282

The number is dependent on the architecture. The above data shows that:
x86	374
x86_64	323

I'm unable to find the history of why it is set to 282 in unistd.h and
when this problem happened.

> 
> I need more details on this number.
> 
> thanks,
> -- Shuah

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Muhammad Usama Anjum]

On 9/18/24 10:46 AM, Muhammad Usama Anjum wrote:
> On 9/17/24 6:56 AM, Shuah Khan wrote:
>> On 9/16/24 00:32, Muhammad Usama Anjum wrote:
>>> On 9/12/24 8:44 PM, Shuah Khan wrote:
>>>> On 9/12/24 04:31, Muhammad Usama Anjum wrote:
>>>>> The value of __NR_userfaultfd was changed to 282 when
>>>>> asm-generic/unistd.h was included. It makes the test to fail every time
>>>>> as the correct number of this syscall on x86_64 is 323. Fix the header
>>>>> to asm/unistd.h.
>>>>>
>>>>
>>>> "please elaborate every time" - I just built on my x86_64 and built
>>>> just fine.
>>> The build isn't broken.
>>>
>>>> I am not saying this isn't a problem, it is good to
>>>> understand why and how it is failing before making the change.
>>> I mean to say that the test is failing at run time because the correct
>>> userfaultfd syscall isn't being found with __NR_userfaultfd = 282.
>>> _NR_userfaultfd's value depends on the header. When asm-generic/unistd.h
>>> is included, its value (282) is wrong. I've tested on x86_64.
>>>
>>
>> Okay - how do you know this is wrong? can you provide more details.
>>
>> git grep _NR_userfaultfd
>> include/uapi/asm-generic/unistd.h:#define __NR_userfaultfd 282
>> include/uapi/asm-generic/unistd.h:__SYSCALL(__NR_userfaultfd,
>> sys_userfaultfd)
>> tools/include/uapi/asm-generic/unistd.h:#define __NR_userfaultfd 282
>>
>>> The fix is simple. Add the correct header which has _NR_userfaultfd =
>>> 323.
> 
> grep -rnIF "#define __NR_userfaultfd"
> tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
> arch/x86/include/generated/uapi/asm/unistd_32.h:374:#define
> __NR_userfaultfd 374
> arch/x86/include/generated/uapi/asm/unistd_64.h:327:#define
> __NR_userfaultfd 323
> arch/x86/include/generated/uapi/asm/unistd_x32.h:282:#define
> __NR_userfaultfd (__X32_SYSCALL_BIT + 323)
> arch/arm/include/generated/uapi/asm/unistd-eabi.h:347:#define
> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
> arch/arm/include/generated/uapi/asm/unistd-oabi.h:359:#define
> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
> include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
> 
> The number is dependent on the architecture. The above data shows that:
> x86	374
> x86_64	323
> 
> I'm unable to find the history of why it is set to 282 in unistd.h and
> when this problem happened.
Does anybody has understanding of this?

> 
>>
>> I need more details on this number.
>>
>> thanks,
>> -- Shuah
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Shuah Khan]

On 9/17/24 23:46, Muhammad Usama Anjum wrote:
> On 9/17/24 6:56 AM, Shuah Khan wrote:
>> On 9/16/24 00:32, Muhammad Usama Anjum wrote:
>>> On 9/12/24 8:44 PM, Shuah Khan wrote:
>>>> On 9/12/24 04:31, Muhammad Usama Anjum wrote:
>>>>> The value of __NR_userfaultfd was changed to 282 when
>>>>> asm-generic/unistd.h was included. It makes the test to fail every time
>>>>> as the correct number of this syscall on x86_64 is 323. Fix the header
>>>>> to asm/unistd.h.
>>>>>
>>>>
>>>> "please elaborate every time" - I just built on my x86_64 and built
>>>> just fine.
>>> The build isn't broken.
>>>
>>>> I am not saying this isn't a problem, it is good to
>>>> understand why and how it is failing before making the change.
>>> I mean to say that the test is failing at run time because the correct
>>> userfaultfd syscall isn't being found with __NR_userfaultfd = 282.
>>> _NR_userfaultfd's value depends on the header. When asm-generic/unistd.h
>>> is included, its value (282) is wrong. I've tested on x86_64.
>>>
>>
>> Okay - how do you know this is wrong? can you provide more details.
>>
>> git grep _NR_userfaultfd
>> include/uapi/asm-generic/unistd.h:#define __NR_userfaultfd 282
>> include/uapi/asm-generic/unistd.h:__SYSCALL(__NR_userfaultfd,
>> sys_userfaultfd)
>> tools/include/uapi/asm-generic/unistd.h:#define __NR_userfaultfd 282
>>
>>> The fix is simple. Add the correct header which has _NR_userfaultfd =
>>> 323.
> 
> grep -rnIF "#define __NR_userfaultfd"
> tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
> arch/x86/include/generated/uapi/asm/unistd_32.h:374:#define
> __NR_userfaultfd 374
> arch/x86/include/generated/uapi/asm/unistd_64.h:327:#define
> __NR_userfaultfd 323
> arch/x86/include/generated/uapi/asm/unistd_x32.h:282:#define
> __NR_userfaultfd (__X32_SYSCALL_BIT + 323)
> arch/arm/include/generated/uapi/asm/unistd-eabi.h:347:#define
> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
> arch/arm/include/generated/uapi/asm/unistd-oabi.h:359:#define
> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
> include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
> 
> The number is dependent on the architecture. The above data shows that:
> x86	374
> x86_64	323

Correct and the generated header files do the right thing and it is good to
include them as this patch does.

This is a good find and fix. I wish you explained this in your changelog.
Please add more details when you send v2.

There could be other issues lurking based on what I found.

The other two files are the problem where they hard code it to 282 without
taking the __NR_SYSCALL_BASE for the arch into consideration:

tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282

> 
> I'm unable to find the history of why it is set to 282 in unistd.h and
> when this problem happened.

According to git history it is added in the following commit to
include/uapi/asm-generic/unistd.h:

09f7298100ea9767324298ab0c7979f6d7463183
Subject: [PATCH] userfaultfd: register uapi generic syscall (aarch64)

and it is added in the following commit to tools/include/uapi/asm-generic/unistd.h
34b009cfde2b8ce20a69c7bfd6bad4ce0e7cd970
Subject: [PATCH] tools include: Grab copies of arm64 dependent unistd.h files

I think, the above defines from include/uapi/asm-generic/unistd.h and
tools/include/uapi/asm-generic/unistd.h should be removed.

Maybe others familiar with userfaultfd can determine the best course of action.
We might have other NR_ defines in these two files that are causing problems
for tests and tools that we haven't uncovered yet.

thanks,
-- Shuah

Re: [PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Muhammad Usama Anjum]

...

>> grep -rnIF "#define __NR_userfaultfd"
>> tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
>> arch/x86/include/generated/uapi/asm/unistd_32.h:374:#define
>> __NR_userfaultfd 374
>> arch/x86/include/generated/uapi/asm/unistd_64.h:327:#define
>> __NR_userfaultfd 323
>> arch/x86/include/generated/uapi/asm/unistd_x32.h:282:#define
>> __NR_userfaultfd (__X32_SYSCALL_BIT + 323)
>> arch/arm/include/generated/uapi/asm/unistd-eabi.h:347:#define
>> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
>> arch/arm/include/generated/uapi/asm/unistd-oabi.h:359:#define
>> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
>> include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
>>
>> The number is dependent on the architecture. The above data shows that:
>> x86    374
>> x86_64    323
> 
> Correct and the generated header files do the right thing and it is good to
> include them as this patch does.
> 
> This is a good find and fix. I wish you explained this in your changelog.
> Please add more details when you send v2.
I'm sending v2

> 
> There could be other issues lurking based on what I found.
> 
> The other two files are the problem where they hard code it to 282 without
> taking the __NR_SYSCALL_BASE for the arch into consideration:
> 
> tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
> include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
> 
>>
>> I'm unable to find the history of why it is set to 282 in unistd.h and
>> when this problem happened.
> 
> According to git history it is added in the following commit to
> include/uapi/asm-generic/unistd.h:
> 
> 09f7298100ea9767324298ab0c7979f6d7463183
> Subject: [PATCH] userfaultfd: register uapi generic syscall (aarch64)
> 
> and it is added in the following commit to
> tools/include/uapi/asm-generic/unistd.h
> 34b009cfde2b8ce20a69c7bfd6bad4ce0e7cd970
> Subject: [PATCH] tools include: Grab copies of arm64 dependent unistd.h
> files
> 
> I think, the above defines from include/uapi/asm-generic/unistd.h and
> tools/include/uapi/asm-generic/unistd.h should be removed.
> 
> Maybe others familiar with userfaultfd can determine the best course of
> action.
> We might have other NR_ defines in these two files that are causing
> problems
> for tests and tools that we haven't uncovered yet.
Added authors of these patches.

> 
> thanks,
> -- Shuah

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Shuah Khan]

On 9/22/24 23:35, Muhammad Usama Anjum wrote:
> ...
> 
>>> grep -rnIF "#define __NR_userfaultfd"
>>> tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
>>> arch/x86/include/generated/uapi/asm/unistd_32.h:374:#define
>>> __NR_userfaultfd 374
>>> arch/x86/include/generated/uapi/asm/unistd_64.h:327:#define
>>> __NR_userfaultfd 323
>>> arch/x86/include/generated/uapi/asm/unistd_x32.h:282:#define
>>> __NR_userfaultfd (__X32_SYSCALL_BIT + 323)
>>> arch/arm/include/generated/uapi/asm/unistd-eabi.h:347:#define
>>> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
>>> arch/arm/include/generated/uapi/asm/unistd-oabi.h:359:#define
>>> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
>>> include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
>>>
>>> The number is dependent on the architecture. The above data shows that:
>>> x86    374
>>> x86_64    323
>>
>> Correct and the generated header files do the right thing and it is good to
>> include them as this patch does.
>>
>> This is a good find and fix. I wish you explained this in your changelog.
>> Please add more details when you send v2.
> I'm sending v2
> 
>>
>> There could be other issues lurking based on what I found.
>>
>> The other two files are the problem where they hard code it to 282 without
>> taking the __NR_SYSCALL_BASE for the arch into consideration:
>>
>> tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
>> include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
>>
>>>
>>> I'm unable to find the history of why it is set to 282 in unistd.h and
>>> when this problem happened.
>>
>> According to git history it is added in the following commit to
>> include/uapi/asm-generic/unistd.h:
>>
>> 09f7298100ea9767324298ab0c7979f6d7463183
>> Subject: [PATCH] userfaultfd: register uapi generic syscall (aarch64)
>>
>> and it is added in the following commit to
>> tools/include/uapi/asm-generic/unistd.h
>> 34b009cfde2b8ce20a69c7bfd6bad4ce0e7cd970
>> Subject: [PATCH] tools include: Grab copies of arm64 dependent unistd.h
>> files
>>
>> I think, the above defines from include/uapi/asm-generic/unistd.h and
>> tools/include/uapi/asm-generic/unistd.h should be removed.
>>
>> Maybe others familiar with userfaultfd can determine the best course of
>> action.
>> We might have other NR_ defines in these two files that are causing
>> problems
>> for tests and tools that we haven't uncovered yet.
> Added authors of these patches.
> 

Thank you. Would you be able top follow up on this and send patches
to remove these defines if it deemed to be the correct solution?

thanks,
-- Shuah

Re: [PATCH 1/2] kselftests: mm: Fix wrong __NR_userfaultfd value

[Muhammad Usama Anjum]

On 9/23/24 9:02 PM, Shuah Khan wrote:
> On 9/22/24 23:35, Muhammad Usama Anjum wrote:
>> ...
>>
>>>> grep -rnIF "#define __NR_userfaultfd"
>>>> tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd
>>>> 282
>>>> arch/x86/include/generated/uapi/asm/unistd_32.h:374:#define
>>>> __NR_userfaultfd 374
>>>> arch/x86/include/generated/uapi/asm/unistd_64.h:327:#define
>>>> __NR_userfaultfd 323
>>>> arch/x86/include/generated/uapi/asm/unistd_x32.h:282:#define
>>>> __NR_userfaultfd (__X32_SYSCALL_BIT + 323)
>>>> arch/arm/include/generated/uapi/asm/unistd-eabi.h:347:#define
>>>> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
>>>> arch/arm/include/generated/uapi/asm/unistd-oabi.h:359:#define
>>>> __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
>>>> include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
>>>>
>>>> The number is dependent on the architecture. The above data shows that:
>>>> x86    374
>>>> x86_64    323
>>>
>>> Correct and the generated header files do the right thing and it is
>>> good to
>>> include them as this patch does.
>>>
>>> This is a good find and fix. I wish you explained this in your
>>> changelog.
>>> Please add more details when you send v2.
>> I'm sending v2
>>
>>>
>>> There could be other issues lurking based on what I found.
>>>
>>> The other two files are the problem where they hard code it to 282
>>> without
>>> taking the __NR_SYSCALL_BASE for the arch into consideration:
>>>
>>> tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
>>> include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
>>>
>>>>
>>>> I'm unable to find the history of why it is set to 282 in unistd.h and
>>>> when this problem happened.
>>>
>>> According to git history it is added in the following commit to
>>> include/uapi/asm-generic/unistd.h:
>>>
>>> 09f7298100ea9767324298ab0c7979f6d7463183
>>> Subject: [PATCH] userfaultfd: register uapi generic syscall (aarch64)
>>>
>>> and it is added in the following commit to
>>> tools/include/uapi/asm-generic/unistd.h
>>> 34b009cfde2b8ce20a69c7bfd6bad4ce0e7cd970
>>> Subject: [PATCH] tools include: Grab copies of arm64 dependent unistd.h
>>> files
>>>
>>> I think, the above defines from include/uapi/asm-generic/unistd.h and
>>> tools/include/uapi/asm-generic/unistd.h should be removed.
>>>
>>> Maybe others familiar with userfaultfd can determine the best course of
>>> action.
>>> We might have other NR_ defines in these two files that are causing
>>> problems
>>> for tests and tools that we haven't uncovered yet.
>> Added authors of these patches.
>>
> 
> Thank you. Would you be able top follow up on this and send patches
> to remove these defines if it deemed to be the correct solution?
Yeah, sure. I'll follow up and fix the issue.

> 
> thanks,
> -- Shuah
> 
> 

-- 
BR,
Muhammad Usama Anjum