On 1/10/23 12:56, David Hildenbrand wrote:
On 10.01.23 12:52, Mirsad Todorovac wrote:
On 1/10/23 12:18, David Hildenbrand wrote:
On 10.01.23 12:00, Mirsad Todorovac wrote:
Hi,
On 1/10/23 00:36, Andrew Morton wrote:
The patch titled
Subject: selftests/mm: define MADV_PAGEOUT to fix compilation
issues
has been added to the -mm mm-unstable branch. Its filename is
selftests-mm-define-madv_pageout-to-fix-compilation-issues.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/selftests-mm-define-madv_pageout-to-fix-compilation-issues.patch
This patch will later appear in the mm-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when
testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: David Hildenbrand <david@xxxxxxxxxx>
Subject: selftests/mm: define MADV_PAGEOUT to fix compilation issues
Date: Mon, 9 Jan 2023 18:12:55 +0100
If MADV_PAGEOUT is not defined (e.g., on AlmaLinux 8), compilation will
fail. Let's fix that like khugepaged.c does by conditionally defining
MADV_PAGEOUT.
Link: https://lkml.kernel.org/r/20230109171255.488749-1-david@xxxxxxxxxx
Fixes: 69c66add5663 ("selftests/vm: anon_cow: test COW handling of
anonymous memory")
Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
Reported-by: Mirsad Goran Todorovac <mirsad.todorovac@xxxxxxxxxxxx>
Cc: Shuah Khan <shuah@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
tools/testing/selftests/mm/cow.c | 3 +++
1 file changed, 3 insertions(+)
---
a/tools/testing/selftests/mm/cow.c~selftests-mm-define-madv_pageout-to-fix-compilation-issues
+++ a/tools/testing/selftests/mm/cow.c
@@ -30,6 +30,9 @@
#include "../kselftest.h"
#include "vm_util.h"
+#ifndef MADV_PAGEOUT
+#define MADV_PAGEOUT 21
+#endif
#ifndef MADV_COLLAPSE
#define MADV_COLLAPSE 25
#endif
_
Patches currently in -mm which might be from david@xxxxxxxxxx are
mm-hugetlb-fix-pte-marker-handling-in-hugetlb_change_protection.patch
mm-hugetlb-fix-uffd-wp-handling-for-migration-entries-in-hugetlb_change_protection.patch
mm-userfaultfd-enable-writenotify-while-userfaultfd-wp-is-enabled-for-a-vma.patch
mm-userfaultfd-rely-on-vma-vm_page_prot-in-uffd_wp_range.patch
mm-userfaultfd-rely-on-vma-vm_page_prot-in-uffd_wp_range-fix.patch
mm-mprotect-drop-pgprot_t-parameter-from-change_protection.patch
mm-mprotect-drop-pgprot_t-parameter-from-change_protection-fix.patch
selftests-vm-cow-add-cow-tests-for-collapsing-of-pte-mapped-anon-thp.patch
mm-nommu-factor-out-check-for-nommu-shared-mappings-into-is_nommu_shared_mapping.patch
mm-nommu-dont-use-vm_mayshare-for-map_private-mappings.patch
drivers-misc-open-dice-dont-touch-vm_mayshare.patch
selftests-mm-define-madv_pageout-to-fix-compilation-issues.patch
Please consider alternative patch instead. I am submitting it for a
review.
It would give the referential integrity to the source of the selftest/vm
subtree.
#ifdef ... #endif encapsulation around `#define __USE_GNU` eliminates a
warning.
RATIONALE:
Though it is ulikely that the values of MADV_PAGEOUT,
MADV_POPULATE_READ and MADV_COLLAPSE will change, the source integrity
would benefit from having it changed in only one place,
.../usr/include/asm-generic/mman-common.h, included by >
#include <linux/mman.h>
Would that imply that we're taking the in-tree sources and not the
installed kernel headers?
Because compilation would fail if the installed kernel headers don't
define these.
It seems that userfaultfd.c already included <linux/mman.h>,
so nothing is lost or gained in compilation successes, except that
things look more uniform, coherent and consistent?
I included it in tools/testing/selftests/mm/madv_populate.c as well BUT that was
not sufficient to make it compile in all environments.
Hi, David,
It's unobvious why, for (as you can witness from the source)
#include <linux/mman.h> includes
#include <asm/mman.h> includes
#include <asm-generic/mman.h> which in turn includes
#include <asm-generic/mman-common.h>
that defines all of the
#define MADV_PAGEOUT 21 /* reclaim these pages */
#define MADV_POPULATE_READ 22 /* populate (prefault) page tables readable */
#define MADV_POPULATE_WRITE 23 /* populate (prefault) page tables writable */
So, everything is within the in-tree includes, so I can't tell what went wrong.
I see that "#include <linux/magic.h>" is already used.
BTW, did userfaultfd.c compile on that Debian 11.5 environment?
commit d88825f22b8f1cad8acb429b6bf0a54c85d7b93f
Author: David Hildenbrand <david@xxxxxxxxxx>
Date: Mon Dec 5 20:37:14 2022 +0100
selftests/vm: madv_populate: fix missing MADV_POPULATE_(READ|WRITE) definitions
The tests fail to compile in some environments (e.g., Debian 11.5 on x86).
On Debian 11 on x86_64 I had no issues (just checked). (Your fix applied.)
Only MADV_PAGEOUT is defined in /usr/include:
mtodorov@magrf:~$ grep -E '(MADV_PAGEOUT|MADV_POPULATE_READ|MADV_POPULATE_WRITE)' -r /usr/include
/usr/include/x86_64-linux-gnu/bits/mman-linux.h:# define MADV_PAGEOUT 21 /* Reclaim these pages. */
/usr/include/asm-generic/mman-common.h:#define MADV_PAGEOUT 21 /* reclaim these pages */
mtodorov@magrf:~$
However, "memfd-secret.c" and "mlock-random-test.c" fail to include <sys/capability.h>
despite being in both /usr/include/linux/capability.h and ../../../../usr/include/sys/capability.h:
mtodorov@magrf:~/linux/kernel/linux_torvalds/tools/testing/selftests/vm$ make -k
gcc -Wall -I /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../.. -I
/home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../../usr/include -isystem
/home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../../usr/include -no-pie memfd_secret.c -lrt
-lpthread -lcap -o /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/vm/memfd_secret
memfd_secret.c:16:10: fatal error: sys/capability.h: No such file or directory
16 | #include <sys/capability.h>
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [../lib.mk:145: /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/vm/migration] Error 1
gcc -Wall -I /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../.. -I
/home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../../usr/include -isystem
/home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../../usr/include -no-pie mlock-random-test.c -lrt
-lpthread -lcap -o /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/vm/mlock-random-test
mlock-random-test.c:8:10: fatal error: sys/capability.h: No such file or directory
8 | #include <sys/capability.h>
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
usr/include/linux/capability.h is existing, but the compiler somehow fails to find it:
mtodorov@magrf:~/linux/kernel/linux_torvalds/tools/testing/selftests/vm$ grep sys/capability.h *.c
memfd_secret.c:#include <sys/capability.h>
mlock-random-test.c:#include <sys/capability.h>
mtodorov@magrf:~/linux/kernel/linux_torvalds/tools/testing/selftests/vm$ find /usr/include -name capability.h
/usr/include/linux/capability.h
mtodorov@magrf:~/linux/kernel/linux_torvalds/tools/testing/selftests/vm$
Let's simply conditionally define MADV_POPULATE_(READ|WRITE) if not
already defined, similar to how the khugepaged.c test handles it.
Link: https://lkml.kernel.org/r/20221205193716.276024-3-david@xxxxxxxxxx
Fixes: 39b2e5cae43d ("selftests/vm: make MADV_POPULATE_(READ|WRITE) use in-tree headers")
Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
Cc: Shuah Khan <shuah@xxxxxxxxxx>
Cc: Yang Li <yang.lee@xxxxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
IMHO, I don't have your experience and knowledge, but from the fresh perspective,
kernel will bring in its own selftest suite all the defines in <linux/mman.h>
that are supported in the particular version.
AFAIK, testing with different version of kernel is not recommended.
Of course, we can easily test with "gcc -E" what exactly gets included.
We don't seem to have a uniform way of handling MADV_PAGEOUT and
MADV_POPULATE(READ|WRITE) even within different C sources within
tools/testing/selftests/vm itself, so it means that we spread the spectrum
of systems and compilations that we will break. :-/
To repeat what is obvious, I am not an authority on the Linux kernel,
but as I said, I have the benefit of a fresh perspective ...
There was an open source proverb that "more eyes see more" :)
Thanks,
Mirsad
--
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia