diff mbox series

[11/12] selftests/mm: fix missing UFFDIO_CONTINUE_MODE_WP and similar build failures

Message ID 20230602013358.900637-12-jhubbard@nvidia.com
State New
Headers
Series A minor flurry of selftest/mm fixes |

Commit Message

John Hubbard June 2, 2023, 1:33 a.m. UTC 
  UFFDIO_CONTINUE_MODE_WP, UFFD_FEATURE_WP_UNPOPULATED, USERFAULTFD_IOC,
and USERFAULTFD_IOC_NEW are needed lately, but they are not in my host
(Arch Linux) distro's userfaultfd.h yet. So put them in here.

A better approach would be to include the uapi version of userfaultfd.h
from the kernel tree, but that currently fails with rather difficult
linker problems (__packed is defined multiple times, ugg), so defer that
to another day and just fix the build for now.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/mm/uffd-common.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

David Hildenbrand June 2, 2023, 10:23 a.m. UTC | #1 
On 02.06.23 03:33, John Hubbard wrote:
> UFFDIO_CONTINUE_MODE_WP, UFFD_FEATURE_WP_UNPOPULATED, USERFAULTFD_IOC,
> and USERFAULTFD_IOC_NEW are needed lately, but they are not in my host
> (Arch Linux) distro's userfaultfd.h yet. So put them in here.
> 
> A better approach would be to include the uapi version of userfaultfd.h
> from the kernel tree, but that currently fails with rather difficult
> linker problems (__packed is defined multiple times, ugg), so defer that
> to another day and just fix the build for now.
> 
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>   tools/testing/selftests/mm/uffd-common.h | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
> index a1cdb78c0762..98847e41ecf9 100644
> --- a/tools/testing/selftests/mm/uffd-common.h
> +++ b/tools/testing/selftests/mm/uffd-common.h
> @@ -36,6 +36,23 @@
>   
>   #define UFFD_FLAGS	(O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY)
>   
> +#ifndef UFFDIO_CONTINUE_MODE_WP
> +#define UFFDIO_CONTINUE_MODE_WP			((__u64)1<<1)
> +#endif
> +
> +#ifndef UFFD_FEATURE_WP_UNPOPULATED
> +#define UFFD_FEATURE_WP_UNPOPULATED		(1<<13)
> +#endif
> +
> +/* ioctls for /dev/userfaultfd */
> +#ifndef USERFAULTFD_IOC
> +#define USERFAULTFD_IOC 0xAA
> +#endif
> +
> +#ifndef USERFAULTFD_IOC_NEW
> +#define USERFAULTFD_IOC_NEW _IO(USERFAULTFD_IOC, 0x00)
> +#endif
> +
>   #define _err(fmt, ...)						\
>   	do {							\
>   		int ret = errno;				\

Unfortunately, that seems to be the ugly way to handle this because
including the in-tree headers seems to not work and I yet haven't
figured out why (there were some changes back and forth so I lost track).

CFLAGS = -Wall -I $(top_srcdir) -I $(top_srcdir)/tools/include/uapi $(EXTRA_CFLAGS) $(KHDR_INCLUDES)


Acked-by: David Hildenbrand <david@redhat.com>
Muhammad Usama Anjum June 2, 2023, 4:25 p.m. UTC | #2 
On 6/2/23 6:33 AM, John Hubbard wrote:
> UFFDIO_CONTINUE_MODE_WP, UFFD_FEATURE_WP_UNPOPULATED, USERFAULTFD_IOC,
> and USERFAULTFD_IOC_NEW are needed lately, but they are not in my host
> (Arch Linux) distro's userfaultfd.h yet. So put them in here.
Selftests are never supposed to build with native header files. Build the
headers in kernel source first. Then building the selftests picks up these
newly built headers by itself. The method to build header files has changed
to `make headers`. The following command builds the mm selftests
successfully every time for me.

make headers && make -C tools/testing/selftests/mm

Please let me know if this doesn't work for you. I'll try to reproduce and fix.

> 
> A better approach would be to include the uapi version of userfaultfd.h
> from the kernel tree, but that currently fails with rather difficult
> linker problems (__packed is defined multiple times, ugg), so defer that
> to another day and just fix the build for now.
> 
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  tools/testing/selftests/mm/uffd-common.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
> index a1cdb78c0762..98847e41ecf9 100644
> --- a/tools/testing/selftests/mm/uffd-common.h
> +++ b/tools/testing/selftests/mm/uffd-common.h
> @@ -36,6 +36,23 @@
>  
>  #define UFFD_FLAGS	(O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY)
>  
> +#ifndef UFFDIO_CONTINUE_MODE_WP
> +#define UFFDIO_CONTINUE_MODE_WP			((__u64)1<<1)
> +#endif
> +
> +#ifndef UFFD_FEATURE_WP_UNPOPULATED
> +#define UFFD_FEATURE_WP_UNPOPULATED		(1<<13)
> +#endif
> +
> +/* ioctls for /dev/userfaultfd */
> +#ifndef USERFAULTFD_IOC
> +#define USERFAULTFD_IOC 0xAA
> +#endif
> +
> +#ifndef USERFAULTFD_IOC_NEW
> +#define USERFAULTFD_IOC_NEW _IO(USERFAULTFD_IOC, 0x00)
> +#endif
> +
>  #define _err(fmt, ...)						\
>  	do {							\
>  		int ret = errno;				\
John Hubbard June 2, 2023, 10:20 p.m. UTC | #3 
On 6/2/23 03:23, David Hildenbrand wrote:
...
> Unfortunately, that seems to be the ugly way to handle this because
> including the in-tree headers seems to not work and I yet haven't
> figured out why (there were some changes back and forth so I lost track).

Yes, ugly, and based on Muhammad's response, I plan on dropping this patch
entirely, in fact.

> 
> CFLAGS = -Wall -I $(top_srcdir) -I $(top_srcdir)/tools/include/uapi $(EXTRA_CFLAGS) $(KHDR_INCLUDES)
> 
> 
> Acked-by: David Hildenbrand <david@redhat.com>
> 

thanks,
John Hubbard June 2, 2023, 10:24 p.m. UTC | #4 
On 6/2/23 09:25, Muhammad Usama Anjum wrote:
> On 6/2/23 6:33 AM, John Hubbard wrote:
>> UFFDIO_CONTINUE_MODE_WP, UFFD_FEATURE_WP_UNPOPULATED, USERFAULTFD_IOC,
>> and USERFAULTFD_IOC_NEW are needed lately, but they are not in my host
>> (Arch Linux) distro's userfaultfd.h yet. So put them in here.
> Selftests are never supposed to build with native header files. Build the

Ah yes, I remember that now. Of course, the problem is that few people
know or remember that, and it's undocumented as well.

> headers in kernel source first. Then building the selftests picks up these
> newly built headers by itself. The method to build header files has changed
> to `make headers`. The following command builds the mm selftests
> successfully every time for me.
> 
> make headers && make -C tools/testing/selftests/mm
> 
> Please let me know if this doesn't work for you. I'll try to reproduce and fix.
> 

Yes thanks. That's a pointer to a full solution, which needs to:

a) automatically invoke "make headers", at least for selftests/mm for
now, and

b) Add something to perhaps Documentation/dev-tools/kselftest.rst to
document this requirement.

I'll work on that.

thanks,
David Hildenbrand June 3, 2023, 8:27 a.m. UTC | #5 
On 03.06.23 00:20, John Hubbard wrote:
> On 6/2/23 03:23, David Hildenbrand wrote:
> ...
>> Unfortunately, that seems to be the ugly way to handle this because
>> including the in-tree headers seems to not work and I yet haven't
>> figured out why (there were some changes back and forth so I lost track).
> 
> Yes, ugly, and based on Muhammad's response, I plan on dropping this patch
> entirely, in fact.

Ah, finally I know why it sometimes worked and sometimes didn't ... yes, 
let's document that somehow.

Maybe we can even warn from the Makefile when the in-tree headers are 
not installed yet?
John Hubbard June 3, 2023, 11:48 p.m. UTC | #6 
On 6/3/23 01:27, David Hildenbrand wrote:
> On 03.06.23 00:20, John Hubbard wrote:
>> On 6/2/23 03:23, David Hildenbrand wrote:
>> ...
>>> Unfortunately, that seems to be the ugly way to handle this because
>>> including the in-tree headers seems to not work and I yet haven't
>>> figured out why (there were some changes back and forth so I lost track).
>>
>> Yes, ugly, and based on Muhammad's response, I plan on dropping this patch
>> entirely, in fact.
> 
> Ah, finally I know why it sometimes worked and sometimes didn't ... yes, 
> let's document that somehow.
> 
> Maybe we can even warn from the Makefile when the in-tree headers are 
> not installed yet?
> 

Yes. And in fact, after some fooling around getting reacquainted with
selftest Makefiles, it turns out that fussy part was setting up to
detect the missing header files. And that seems to be working reliably
now.

So from there, it's just as easy to automatically build any missing
header files and continue as it is to warn about it.

I'll post a patch.


thanks,
diff mbox series

Patch

diff --git a/tools/testing/selftests/mm/uffd-common.h b/tools/testing/selftests/mm/uffd-common.h
index a1cdb78c0762..98847e41ecf9 100644
--- a/tools/testing/selftests/mm/uffd-common.h
+++ b/tools/testing/selftests/mm/uffd-common.h
@@ -36,6 +36,23 @@ 
 
 #define UFFD_FLAGS	(O_CLOEXEC | O_NONBLOCK | UFFD_USER_MODE_ONLY)
 
+#ifndef UFFDIO_CONTINUE_MODE_WP
+#define UFFDIO_CONTINUE_MODE_WP			((__u64)1<<1)
+#endif
+
+#ifndef UFFD_FEATURE_WP_UNPOPULATED
+#define UFFD_FEATURE_WP_UNPOPULATED		(1<<13)
+#endif
+
+/* ioctls for /dev/userfaultfd */
+#ifndef USERFAULTFD_IOC
+#define USERFAULTFD_IOC 0xAA
+#endif
+
+#ifndef USERFAULTFD_IOC_NEW
+#define USERFAULTFD_IOC_NEW _IO(USERFAULTFD_IOC, 0x00)
+#endif
+
 #define _err(fmt, ...)						\
 	do {							\
 		int ret = errno;				\