[RFC PATCH 4/8] mm/madvise: define madvise behavior in a struct

From: Nadav Amit
Date: Sun Sep 26 2021 - 19:44:06 EST


From: Nadav Amit <namit@xxxxxxxxxx>

The different behaviors of madvise are different in several ways, which
are distributed across several functions. Use the design pattern from
iouring in order to define the actions that are required for each
behavior.

The next patches will get rid of old helper functions that are modified
in this patch and the redundant use of array_index_nospec(). The next
patches will add more actions for each leaf into the new struct.

No functional change is intended.

Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Minchan Kim <minchan@xxxxxxxxxx>
Cc: Colin Cross <ccross@xxxxxxxxxx>
Cc: Suren Baghdasarya <surenb@xxxxxxxxxx>
Cc: Mike Rapoport <rppt@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Nadav Amit <namit@xxxxxxxxxx>
---
mm/madvise.c | 168 +++++++++++++++++++++++++++++++++------------------
1 file changed, 109 insertions(+), 59 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 17e39c70704b..127507c71ba9 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -29,6 +29,7 @@
#include <linux/swapops.h>
#include <linux/shmem_fs.h>
#include <linux/mmu_notifier.h>
+#include <linux/nospec.h>

#include <asm/tlb.h>

@@ -39,6 +40,101 @@ struct madvise_walk_private {
bool pageout;
};

+struct madvise_info {
+ u8 behavior_valid: 1;
+ u8 process_behavior_valid: 1;
+ u8 need_mmap_read_only: 1;
+};
+
+static const struct madvise_info madvise_info[MADV_SOFT_OFFLINE+1] = {
+ [MADV_DOFORK] = {
+ .behavior_valid = 1,
+ },
+ [MADV_DONTFORK] = {
+ .behavior_valid = 1,
+ },
+ [MADV_NORMAL] = {
+ .behavior_valid = 1,
+ },
+ [MADV_SEQUENTIAL] = {
+ .behavior_valid = 1,
+ },
+ [MADV_RANDOM] = {
+ .behavior_valid = 1,
+ },
+ [MADV_REMOVE] = {
+ .behavior_valid = 1,
+ .need_mmap_read_only = 1,
+ },
+ [MADV_WILLNEED] = {
+ .behavior_valid = 1,
+ .process_behavior_valid = 1,
+ .need_mmap_read_only = 1,
+ },
+ [MADV_DONTNEED] = {
+ .behavior_valid = 1,
+ .need_mmap_read_only = 1,
+ },
+ [MADV_FREE] = {
+ .behavior_valid = 1,
+ .need_mmap_read_only = 1,
+ },
+ [MADV_COLD] = {
+ .behavior_valid = 1,
+ .process_behavior_valid = 1,
+ .need_mmap_read_only = 1,
+ },
+ [MADV_PAGEOUT] = {
+ .behavior_valid = 1,
+ .process_behavior_valid = 1,
+ .need_mmap_read_only = 1,
+ },
+#ifdef CONFIG_KSM
+ [MADV_MERGEABLE] = {
+ .behavior_valid = 1,
+ },
+ [MADV_UNMERGEABLE] = {
+ .behavior_valid = 1,
+ },
+#endif
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+ [MADV_HUGEPAGE] = {
+ .behavior_valid = 1,
+ },
+ [MADV_NOHUGEPAGE] = {
+ .behavior_valid = 1,
+ },
+#endif
+ [MADV_DONTDUMP] = {
+ .behavior_valid = 1,
+ },
+ [MADV_DODUMP] = {
+ .behavior_valid = 1,
+ },
+ [MADV_WIPEONFORK] = {
+ .behavior_valid = 1,
+ },
+ [MADV_KEEPONFORK] = {
+ .behavior_valid = 1,
+ },
+#ifdef CONFIG_MEMORY_FAILURE
+ [MADV_HWPOISON] = {
+ .behavior_valid = 1,
+ },
+ [MADV_SOFT_OFFLINE] = {
+ .behavior_valid = 1,
+ },
+#endif
+ [MADV_POPULATE_READ] = {
+ .behavior_valid = 1,
+ .need_mmap_read_only = 1,
+ },
+ [MADV_POPULATE_WRITE] = {
+ .behavior_valid = 1,
+ .need_mmap_read_only = 1,
+ },
+};
+
/*
* Any behaviour which results in changes to the vma->vm_flags needs to
* take mmap_lock for writing. Others, which simply traverse vmas, need
@@ -46,20 +142,7 @@ struct madvise_walk_private {
*/
static int madvise_need_mmap_write(int behavior)
{
- switch (behavior) {
- case MADV_REMOVE:
- case MADV_WILLNEED:
- case MADV_DONTNEED:
- case MADV_COLD:
- case MADV_PAGEOUT:
- case MADV_FREE:
- case MADV_POPULATE_READ:
- case MADV_POPULATE_WRITE:
- return 0;
- default:
- /* be safe, default to 1. list exceptions explicitly */
- return 1;
- }
+ return !madvise_info[behavior].need_mmap_read_only;
}

/*
@@ -999,56 +1082,23 @@ madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
}

static bool
-madvise_behavior_valid(int behavior)
+madvise_behavior_valid(int *behavior)
{
- switch (behavior) {
- case MADV_DOFORK:
- case MADV_DONTFORK:
- case MADV_NORMAL:
- case MADV_SEQUENTIAL:
- case MADV_RANDOM:
- case MADV_REMOVE:
- case MADV_WILLNEED:
- case MADV_DONTNEED:
- case MADV_FREE:
- case MADV_COLD:
- case MADV_PAGEOUT:
- case MADV_POPULATE_READ:
- case MADV_POPULATE_WRITE:
-#ifdef CONFIG_KSM
- case MADV_MERGEABLE:
- case MADV_UNMERGEABLE:
-#endif
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
- case MADV_HUGEPAGE:
- case MADV_NOHUGEPAGE:
-#endif
- case MADV_DONTDUMP:
- case MADV_DODUMP:
- case MADV_WIPEONFORK:
- case MADV_KEEPONFORK:
-#ifdef CONFIG_MEMORY_FAILURE
- case MADV_SOFT_OFFLINE:
- case MADV_HWPOISON:
-#endif
- return true;
-
- default:
+ if (*behavior >= ARRAY_SIZE(madvise_info))
return false;
- }
+
+ *behavior = array_index_nospec(*behavior, ARRAY_SIZE(madvise_info));
+ return madvise_info[*behavior].behavior_valid;
}

static bool
-process_madvise_behavior_valid(int behavior)
+process_madvise_behavior_valid(int *behavior)
{
- switch (behavior) {
- case MADV_COLD:
- case MADV_PAGEOUT:
- case MADV_WILLNEED:
- return true;
- default:
+ if (*behavior >= ARRAY_SIZE(madvise_info))
return false;
- }
+
+ *behavior = array_index_nospec(*behavior, ARRAY_SIZE(madvise_info));
+ return madvise_info[*behavior].process_behavior_valid;
}

/*
@@ -1133,7 +1183,7 @@ int do_madvise(struct mm_struct *mm, unsigned long start, size_t len_in, int beh

start = untagged_addr(start);

- if (!madvise_behavior_valid(behavior))
+ if (!madvise_behavior_valid(&behavior))
return error;

if (!PAGE_ALIGNED(start))
@@ -1258,7 +1308,7 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
goto put_pid;
}

- if (!process_madvise_behavior_valid(behavior)) {
+ if (!process_madvise_behavior_valid(&behavior)) {
ret = -EINVAL;
goto release_task;
}
--
2.25.1